From mboxrd@z Thu Jan 1 00:00:00 1970 From: Namhyung Kim Subject: Re: [PATCH 6/7] qemu: Implement virtio-pstore device Date: Thu, 28 Jul 2016 14:39:53 +0900 Message-ID: <20160728053953.GB4371@sejong> References: <1469632111-23260-1-git-send-email-namhyung@kernel.org> <1469632111-23260-7-git-send-email-namhyung@kernel.org> <20160728023254-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20160728023254-mutt-send-email-mst@kernel.org> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: "Michael S. Tsirkin" Cc: Tony Luck , Kees Cook , kvm@vger.kernel.org, Radim =?utf-8?B?S3LEjW3DocWZ?= , Anton Vorontsov , LKML , Steven Rostedt , qemu-devel@nongnu.org, Minchan Kim , Anthony Liguori , Colin Cross , Paolo Bonzini , virtualization@lists.linux-foundation.org, Ingo Molnar List-Id: virtualization@lists.linuxfoundation.org T24gVGh1LCBKdWwgMjgsIDIwMTYgYXQgMDM6MDI6NTRBTSArMDMwMCwgTWljaGFlbCBTLiBUc2ly a2luIHdyb3RlOgo+IE9uIFRodSwgSnVsIDI4LCAyMDE2IGF0IDEyOjA4OjMwQU0gKzA5MDAsIE5h bWh5dW5nIEtpbSB3cm90ZToKPiA+IEFkZCB2aXJ0aW8gcHN0b3JlIGRldmljZSB0byBhbGxvdyBr ZXJuZWwgbG9nIGZpbGVzIHNhdmVkIG9uIHRoZSBob3N0Lgo+ID4gSXQgd2lsbCBzYXZlIHRoZSBs b2cgZmlsZXMgb24gdGhlIGRpcmVjdG9yeSBnaXZlbiBieSBwc3RvcmUgZGV2aWNlCj4gPiBvcHRp b24uCj4gPiAKPiA+ICAgJCBxZW11LXN5c3RlbS14ODZfNjQgLWRldmljZSB2aXJ0aW8tcHN0b3Jl LGRpcmVjdG9yeT1kaXIteHggLi4uCj4gPiAKPiA+ICAgKGd1ZXN0KSAjIGVjaG8gYyA+IC9wcm9j L3N5c3JxLXRyaWdnZXIKPiAKPiBTbyBpZiB0aGUgcG9pbnQgaXMgaGFuZGxpbmcgc3lzdGVtIGNy YXNoZXMsIEkgc3VzcGVjdAo+IHZpcnRpbyBpcyB0aGUgd3JvbmcgcHJvdG9jb2wgdG8gdXNlLiBB VE0gaXQncyByYXRoZXIKPiBlbGFib3JhdGUgZm9yIHBlcmZvcm1hbmNlLCBpdCdzIGxpa2VseSBu b3QgdG8gd29yayB3aGVuCj4gbGludXggaXMgY3Jhc2hpbmcuIEkgdGhpbmsgeW91IHdhbnQgc29t ZXRoaW5nIHZlcnkgdmVyeQo+IHNpbXBsZSB0aGF0IHdpbGwgc3RpbGwgd29yayB3aGVuIHlvdSBj cmFzaC4gTGlrZSBtYXliZQo+IGEgc2VyaWFsIGRldmljZT8KCldlbGwsIEkgZG9udCcga25vdy4g IEFzIHlvdSBrbm93LCB0aGUga2VybmVsIG9vcHMgZHVtcCBpcyBhbHJlYWR5IHNlbnQKdG8gc2Vy aWFsIGRldmljZSBidXQgaXQncyByYXRoZXIgc2xvdy4gIEFzIEkgd3JvdGUgaW4gdGhlIGNvdmVy CmxldHRlciwgZW5hYmxpbmcgZnRyYWNlX2R1bXBfb25fb29wcyBtYWtlcyBpdCBldmVuIHdvcnNl Li4gIEFsc28KcHN0b3JlIHNhdmVzIHRoZSAoY29tcHJlc3NlZCkgYmluYXJ5IGRhdGEgc28gSSB0 aG91Z2h0IGl0J2QgYmUgYmV0dGVyCnRvIGhhdmUgYSBkZWRpY2F0ZWQgSU8gY2hhbm5lbC4KCkkg a25vdyB0aGF0IHdlIGNhbid0IHJlbHkgb24gYW55dGhpbmcgaW4ga2VybmVsIHdoZW4gdGhlIGtl cm5lbCBpcwpjcmFzaGluZy4gIEluIHRoZSB2aXJ0dWFsaXphdGlvbiBlbnZpcm9ubWVudCwgSSB0 aGluayBpdCdkIGJlIGdyZWF0IGlmCml0IGNhbiBkdW1wIHRoZSBjcmFzaCBkYXRhIGluIHRoZSBo b3N0IGRpcmVjdGx5IHNvIEkgY2hvc2UgdGhlIHZpcnRpby4KSSB0aG91Z2h0IHRoZSB2aXJ0aW8g aXMgYSByZWxhdGl2ZWx5IHRoaW4gbGF5ZXIgYW5kIGluZGVwZW5kZW50IGZyb20Kb3RoZXIga2Vy bmVsIGZlYXR1cmVzLiAgRXZlbiBpZiBpdCBkb2Vzbid0IGd1YXJhbnRlZSB0byB3b3JrIDEwMCUs Cml0J3Mgd29ydGggdHJ5aW5nIElNSE8uCgoKPiAKPiA+ICAgJCBscyBkaXIteHgKPiA+ICAgZG1l c2ctMS5lbmMueiAgZG1lc2ctMi5lbmMuego+ID4gCj4gPiBUaGUgbG9nIGZpbGVzIGFyZSB1c3Vh bGx5IGNvbXByZXNzZWQgdXNpbmcgemxpYi4gIFVzZXJzIGNhbiBzZWUgdGhlIGxvZwo+ID4gbWVz c2FnZXMgZGlyZWN0bHkgb24gdGhlIGhvc3Qgb3Igb24gdGhlIGd1ZXN0ICh1c2luZyBwc3RvcmUg ZmlsZXN5c3RlbSkuCj4gCj4gU28gdGhpcyBsYWNrcyBhbGwgbWFuYWdlbWVudCB0b29scyB0aGF0 IGEgcmVndWxhciBzdG9yYWdlIGRldmljZQo+IGhhcywgYW5kIHRoZXNlIGFyZSBuZWNlc3Nhcnkg aWYgcGVvcGxlIGFyZSB0byB1c2UgdGhpcwo+IGluIHByb2R1Y3Rpb24uCj4gCj4gRm9yIGV4YW1w bGUsIHNvbWUga2luZCBvZiBwcm92aXNpb24gZm9yIGxpbWl0aW5nIHRoZSBhbW91bnQgb2YgaG9z dCBkaXNrCj4gdGhpcyBjYW4gY29uc3VtZSBzZWVtcyBjYWxsZWQgZm9yLiBSYXRlLWxpbWl0aW5n IGRpc2sgd3JpdGVzCj4gb24gaG9zdCBhbHNvIHNlZW1zIG5lY2Vzc2FyeS4gSGFuZGxpbmcgaG9z dCBkaXNrIGVycm9ycyBhbHdheXMKPiBwcm9wYWdhdGVzIGVycm9yIHRvIGd1ZXN0IGJ1dCBhbiBv cHRpb24gdG8gZS5nLiBzdG9wIHZtIG1pZ2h0Cj4gYmUgdXNlZnVsIHRvIGF2b2lkIGNvcnJ1cHRp b24uCgpZZXMsIGl0IG5lZWRzIHRoYXQga2luZCBvZiBtYW5hZ2VtZW50LiAgSSdkIGxpa2UgdG8g bG9vayBhdCB0aGUKZXhpc3RpbmcgaW1wbGVtZW50YXRpb24uICBCdXQgYmFzaWNhbGx5IEkgdGhv dWdodCBpdCBhcyBhIGRlYnVnZ2luZwpmZWF0dXJlIGFuZCBpdCB3b24ndCBwcm9kdWNlIG11Y2gg ZGF0YSBmb3IgdGhlIGRlZmF1bHQgc2V0dGluZy4gIE1heWJlCkkgY2FuIGxpbWl0IHRoZSBmaWxl IHNpemUgbm90IHRvIGV4Y2VlZCB0aGUgYnVmZmVyIHNpemUgYW5kIGtlZXAgdXAgdG8KcHJlLWNv bmZpZ3VyZWQgbnVtYmVyIG9mIGZpbGVzIGZvciBlYWNoIHR5cGUuICBOb3cgaG9zdCBkaXNrIGVy cm9yCndpbGwgYmUgcHJvcGFnYXRlZCB0byBndWVzdC4KCgo+IAo+ID4gCj4gPiBUaGUgJ2RpcmVj dG9yeScgcHJvcGVydHkgaXMgcmVxdWlyZWQgZm9yIHZpcnRpby1wc3RvcmUgZGV2aWNlIHRvIHdv cmsuCj4gPiBJdCBhbHNvIGFkZHMgJ2J1ZnNpemUnIGFuZCAnY29uc29sZScgKGJvb2xlYW4pIHBy b3BlcnRpZXMuCj4gCj4gTm8gaWRlYSB3aGF0IHRoZXNlIGRvLiBTZWVtIHRvIGJlIFJXIGJ5IGd1 ZXN0IGJ1dCBoYXZlIG5vIGVmZmVjdAo+IG90aGVyd2lzZS4KClRoZSAnZGlyZWN0b3J5JyBpcyBh IHBhdGhuYW1lIHdoaWNoIGl0IHNhdmVzIHRoZSBkYXRhIGZpbGVzLiAgVGhlCididWZzaXplJyBz ZXRzIHRoZSBzaXplIG9mIGJ1ZmZlciB1c2VkIGJ5IHBzdG9yZS4gIFRoZSAnY29uc29sZScKZW5h YmxlcyBzYXZpbmcgcHN0b3JlIGNvbnNvbGUgdHlwZSBkYXRhLgoKCj4gCj4gCj4gPiBDYzogUGFv bG8gQm9uemluaSA8cGJvbnppbmlAcmVkaGF0LmNvbT4KPiA+IENjOiBSYWRpbSBLcsSNbcOhxZkg PHJrcmNtYXJAcmVkaGF0LmNvbT4KPiA+IENjOiAiTWljaGFlbCBTLiBUc2lya2luIiA8bXN0QHJl ZGhhdC5jb20+Cj4gPiBDYzogQW50aG9ueSBMaWd1b3JpIDxhbGlndW9yaUBhbWF6b24uY29tPgo+ ID4gQ2M6IEFudG9uIFZvcm9udHNvdiA8YW50b25AZW5vbXNnLm9yZz4KPiA+IENjOiBDb2xpbiBD cm9zcyA8Y2Nyb3NzQGFuZHJvaWQuY29tPgo+ID4gQ2M6IEtlZXMgQ29vayA8a2Vlc2Nvb2tAY2hy b21pdW0ub3JnPgo+ID4gQ2M6IFRvbnkgTHVjayA8dG9ueS5sdWNrQGludGVsLmNvbT4KPiA+IENj OiBTdGV2ZW4gUm9zdGVkdCA8cm9zdGVkdEBnb29kbWlzLm9yZz4KPiA+IENjOiBJbmdvIE1vbG5h ciA8bWluZ29Aa2VybmVsLm9yZz4KPiA+IENjOiBNaW5jaGFuIEtpbSA8bWluY2hhbkBrZXJuZWwu b3JnPgo+ID4gQ2M6IGt2bUB2Z2VyLmtlcm5lbC5vcmcKPiA+IENjOiBxZW11LWRldmVsQG5vbmdu dS5vcmcKPiA+IENjOiB2aXJ0dWFsaXphdGlvbkBsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwo+ ID4gU2lnbmVkLW9mZi1ieTogTmFtaHl1bmcgS2ltIDxuYW1oeXVuZ0BrZXJuZWwub3JnPgo+ID4g LS0tCgpbU05JUF0KPiA+IGRpZmYgLS1naXQgYS9ody92aXJ0aW8vdmlydGlvLXBzdG9yZS5jIGIv aHcvdmlydGlvL3ZpcnRpby1wc3RvcmUuYwo+ID4gbmV3IGZpbGUgbW9kZSAxMDA2NDQKPiA+IGlu ZGV4IDAwMDAwMDAuLjJjYTc3ODYKPiA+IC0tLSAvZGV2L251bGwKPiA+ICsrKyBiL2h3L3ZpcnRp by92aXJ0aW8tcHN0b3JlLmMKPiA+IEBAIC0wLDAgKzEsNDc3IEBACj4gPiArLyoKPiA+ICsgKiBW aXJ0aW8gUHN0b3JlIERldmljZQo+ID4gKyAqCj4gPiArICogQ29weXJpZ2h0IChDKSAyMDE2ICBM RyBFbGVjdHJvbmljcwo+ID4gKyAqCj4gPiArICogQXV0aG9yczoKPiA+ICsgKiAgTmFtaHl1bmcg S2ltICA8bmFtaHl1bmdAZ21haWwuY29tPgo+ID4gKyAqCj4gPiArICogVGhpcyB3b3JrIGlzIGxp Y2Vuc2VkIHVuZGVyIHRoZSB0ZXJtcyBvZiB0aGUgR05VIEdQTCwgdmVyc2lvbiAyLgo+IAo+IFdl IGdlbmVyYWxseSBhc2sgbmV3IGNvZGUgdG8gYmUgdjIgb3IgbGF0ZXIuCgpPaywgd2lsbCB1cGRh dGUuCgoKPiAKPiA+ICBTZWUKPiA+ICsgKiB0aGUgQ09QWUlORyBmaWxlIGluIHRoZSB0b3AtbGV2 ZWwgZGlyZWN0b3J5Lgo+ID4gKyAqCj4gPiArICovCj4gPiArCj4gPiArI2luY2x1ZGUgPHN0ZGlv Lmg+Cj4gPiArCj4gPiArI2luY2x1ZGUgInFlbXUvb3NkZXAuaCIKPiA+ICsjaW5jbHVkZSAicWVt dS9pb3YuaCIKPiA+ICsjaW5jbHVkZSAicWVtdS1jb21tb24uaCIKPiA+ICsjaW5jbHVkZSAicWVt dS9jdXRpbHMuaCIKPiA+ICsjaW5jbHVkZSAicWVtdS9lcnJvci1yZXBvcnQuaCIKPiA+ICsjaW5j bHVkZSAic3lzZW11L2t2bS5oIgo+ID4gKyNpbmNsdWRlICJxYXBpL3Zpc2l0b3IuaCIKPiA+ICsj aW5jbHVkZSAicWFwaS1ldmVudC5oIgo+ID4gKyNpbmNsdWRlICJ0cmFjZS5oIgo+ID4gKwo+ID4g KyNpbmNsdWRlICJody92aXJ0aW8vdmlydGlvLmgiCj4gPiArI2luY2x1ZGUgImh3L3ZpcnRpby92 aXJ0aW8tYnVzLmgiCj4gPiArI2luY2x1ZGUgImh3L3ZpcnRpby92aXJ0aW8tYWNjZXNzLmgiCj4g PiArI2luY2x1ZGUgImh3L3ZpcnRpby92aXJ0aW8tcHN0b3JlLmgiCj4gPiArCj4gPiArCj4gPiAr c3RhdGljIHZvaWQgdmlydGlvX3BzdG9yZV90b19maWxlbmFtZShWaXJ0SU9Qc3RvcmUgKnMsIGNo YXIgKmJ1Ziwgc2l6ZV90IHN6LAo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgc3RydWN0IHZpcnRpb19wc3RvcmVfcmVxICpyZXEpCj4gPiArewo+ID4gKyAgICBjb25z dCBjaGFyICpiYXNlbmFtZTsKPiA+ICsgICAgdW5zaWduZWQgbG9uZyBsb25nIGlkID0gMDsKPiA+ ICsgICAgdW5zaWduZWQgaW50IGZsYWdzID0gbGUzMl90b19jcHUocmVxLT5mbGFncyk7Cj4gPiAr Cj4gPiArICAgIHN3aXRjaCAobGUxNl90b19jcHUocmVxLT50eXBlKSkgewo+ID4gKyAgICBjYXNl IFZJUlRJT19QU1RPUkVfVFlQRV9ETUVTRzoKPiA+ICsgICAgICAgIGJhc2VuYW1lID0gImRtZXNn IjsKPiA+ICsgICAgICAgIGlkID0gcy0+aWQrKzsKPiA+ICsgICAgICAgIGJyZWFrOwo+ID4gKyAg ICBjYXNlIFZJUlRJT19QU1RPUkVfVFlQRV9DT05TT0xFOgo+ID4gKyAgICAgICAgYmFzZW5hbWUg PSAiY29uc29sZSI7Cj4gPiArICAgICAgICBpZiAocy0+Y29uc29sZV9pZCkgewo+ID4gKyAgICAg ICAgICAgIGlkID0gcy0+Y29uc29sZV9pZDsKPiA+ICsgICAgICAgIH0gZWxzZSB7Cj4gPiArICAg ICAgICAgICAgaWQgPSBzLT5jb25zb2xlX2lkID0gcy0+aWQrKzsKPiA+ICsgICAgICAgIH0KPiA+ ICsgICAgICAgIGJyZWFrOwo+ID4gKyAgICBkZWZhdWx0Ogo+ID4gKyAgICAgICAgYmFzZW5hbWUg PSAidW5rbm93biI7Cj4gPiArICAgICAgICBicmVhazsKPiA+ICsgICAgfQo+ID4gKwo+ID4gKyAg ICBzbnByaW50ZihidWYsIHN6LCAiJXMvJXMtJWxsdSVzIiwgcy0+ZGlyZWN0b3J5LCBiYXNlbmFt ZSwgaWQsCj4gPiArICAgICAgICAgICAgIGZsYWdzICYgVklSVElPX1BTVE9SRV9GTF9DT01QUkVT U0VEID8gIi5lbmMueiIgOiAiIik7Cj4gPiArfQo+ID4gKwo+ID4gK3N0YXRpYyB2b2lkIHZpcnRp b19wc3RvcmVfZnJvbV9maWxlbmFtZShWaXJ0SU9Qc3RvcmUgKnMsIGNoYXIgKm5hbWUsCj4gPiAr ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNoYXIgKmJ1Ziwgc2l6ZV90 IHN6LAo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdHJ1Y3Qg dmlydGlvX3BzdG9yZV9maWxlaW5mbyAqaW5mbykKPiA+ICt7Cj4gPiArICAgIHNucHJpbnRmKGJ1 Ziwgc3osICIlcy8lcyIsIHMtPmRpcmVjdG9yeSwgbmFtZSk7Cj4gCj4gaWYgdGhpcyBkb2VzIG5v dCBmaXQsIGJ1ZiB3aWxsIG5vdCBiZSAwIHRlcm1pbmF0ZWQgYW5kIGNhbgo+IGdlbmVyYWxseSBi ZSBjb3JydXB0ZWQuCgpXaWxsIGNoYW5nZSBpdCB0byB1c2UgbWFsbG9jIGluc3RlYWQuCgo+IAo+ IAo+ID4gKwo+ID4gKyAgICBpZiAoZ19zdHJfaGFzX3ByZWZpeChuYW1lLCAiZG1lc2ctIikpIHsK PiA+ICsgICAgICAgIGluZm8tPnR5cGUgPSBWSVJUSU9fUFNUT1JFX1RZUEVfRE1FU0c7Cj4gPiAr ICAgICAgICBuYW1lICs9IHN0cmxlbigiZG1lc2ctIik7Cj4gPiArICAgIH0gZWxzZSBpZiAoZ19z dHJfaGFzX3ByZWZpeChuYW1lLCAiY29uc29sZS0iKSkgewo+ID4gKyAgICAgICAgaW5mby0+dHlw ZSA9IFZJUlRJT19QU1RPUkVfVFlQRV9DT05TT0xFOwo+ID4gKyAgICAgICAgbmFtZSArPSBzdHJs ZW4oImNvbnNvbGUtIik7Cj4gPiArICAgIH0gZWxzZSBpZiAoZ19zdHJfaGFzX3ByZWZpeChuYW1l LCAidW5rbm93bi0iKSkgewo+ID4gKyAgICAgICAgaW5mby0+dHlwZSA9IFZJUlRJT19QU1RPUkVf VFlQRV9VTktOT1dOOwo+ID4gKyAgICAgICAgbmFtZSArPSBzdHJsZW4oInVua25vd24tIik7Cj4g PiArICAgIH0KPiA+ICsKPiA+ICsgICAgcWVtdV9zdHJ0b3VsbChuYW1lLCBOVUxMLCAwLCAmaW5m by0+aWQpOwo+ID4gKwo+ID4gKyAgICBpbmZvLT5mbGFncyA9IDA7Cj4gPiArICAgIGlmIChnX3N0 cl9oYXNfc3VmZml4KG5hbWUsICIuZW5jLnoiKSkgewo+ID4gKyAgICAgICAgaW5mby0+ZmxhZ3Mg fD0gVklSVElPX1BTVE9SRV9GTF9DT01QUkVTU0VEOwo+ID4gKyAgICB9Cj4gPiArfQoKW1NOSVBd Cj4gPiArc3RhdGljIHNzaXplX3QgdmlydGlvX3BzdG9yZV9kb193cml0ZShWaXJ0SU9Qc3RvcmUg KnMsIHN0cnVjdCBpb3ZlYyAqb3V0X3NnLAo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgdW5zaWduZWQgaW50IG91dF9udW0sCj4gPiArICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgdmlydGlvX3BzdG9yZV9yZXEgKnJlcSkKPiA+ICt7 Cj4gPiArICAgIGNoYXIgcGF0aFtQQVRIX01BWF07Cj4gPiArICAgIGludCBmZDsKPiA+ICsgICAg c3NpemVfdCBsZW47Cj4gPiArICAgIHVuc2lnbmVkIHNob3J0IHR5cGU7Cj4gPiArICAgIGludCBm bGFncyA9IE9fV1JPTkxZIHwgT19DUkVBVDsKPiA+ICsKPiA+ICsgICAgLyogd2UgYWxyZWFkeSBj b25zdW1lIHRoZSByZXEgKi8KPiA+ICsgICAgaW92X2Rpc2NhcmRfZnJvbnQoJm91dF9zZywgJm91 dF9udW0sIHNpemVvZigqcmVxKSk7Cj4gPiArCj4gPiArICAgIHZpcnRpb19wc3RvcmVfdG9fZmls ZW5hbWUocywgcGF0aCwgc2l6ZW9mKHBhdGgpLCByZXEpOwo+ID4gKwo+ID4gKyAgICB0eXBlID0g bGUxNl90b19jcHUocmVxLT50eXBlKTsKPiA+ICsKPiA+ICsgICAgaWYgKHR5cGUgPT0gVklSVElP X1BTVE9SRV9UWVBFX0RNRVNHKSB7Cj4gPiArICAgICAgICBmbGFncyB8PSBPX1RSVU5DOwo+ID4g KyAgICB9IGVsc2UgaWYgKHR5cGUgPT0gVklSVElPX1BTVE9SRV9UWVBFX0NPTlNPTEUpIHsKPiA+ ICsgICAgICAgIGZsYWdzIHw9IE9fQVBQRU5EOwo+ID4gKyAgICB9Cj4gPiArCj4gPiArICAgIGZk ID0gb3BlbihwYXRoLCBmbGFncywgMDY0NCk7Cj4gPiArICAgIGlmIChmZCA8IDApIHsKPiA+ICsg ICAgICAgIGVycm9yX3JlcG9ydCgiY2Fubm90IG9wZW4gJXMiLCBwYXRoKTsKPiA+ICsgICAgICAg IHJldHVybiAtMTsKPiA+ICsgICAgfQo+ID4gKyAgICBsZW4gPSB3cml0ZXYoZmQsIG91dF9zZywg b3V0X251bSk7Cj4gPiArICAgIGNsb3NlKGZkKTsKPiA+ICsKPiA+ICsgICAgcmV0dXJuIGxlbjsK PiAKPiBBbGwgdGhpcyBpcyBibG9ja2luZyBWTSB1bnRpbCBob3N0IGlvIGNvbXBsZXRlcy4KCkht bS4uIEkgZG9uJ3Qga25vdyBhYm91dCB0aGUgaW50ZXJuYWxzIG9mIHFlbXUuICBTbyBkb2VzIGl0 IG1ha2UgZ3Vlc3QKc3RvcD8gIElmIHNvLCB0aGF0J3Mgd2hhdCBJIHdhbnQgdG8gZG8gZm9yIF9E TUVTRy4gOikgIEFzIGl0J3MgY2FsbGVkCm9ubHkgb24ga2VybmVsIG9vcHMgSSB0aGluayBpdCdz IGFkbWl0dGFibGUuICBCdXQgZm9yIF9DT05TT0xFLCBpdApuZWVkcyB0byBkbyBhc3luY2hyb25v dXNseS4gIE1heWJlIEkgY2FuIGFkZCBhIHRocmVhZCB0byBkbyB0aGUgd29yay4KCgo+IAo+IAo+ ID4gK30KPiA+ICsKPiA+ICtzdGF0aWMgc3NpemVfdCB2aXJ0aW9fcHN0b3JlX2RvX2Nsb3NlKFZp cnRJT1BzdG9yZSAqcykKPiA+ICt7Cj4gPiArICAgIGlmIChzLT5kaXJwID09IE5VTEwpIHsKPiA+ ICsgICAgICAgIHJldHVybiAwOwo+ID4gKyAgICB9Cj4gPiArCj4gPiArICAgIGNsb3NlZGlyKHMt PmRpcnApOwo+ID4gKyAgICBzLT5kaXJwID0gTlVMTDsKPiA+ICsKPiA+ICsgICAgcmV0dXJuIDA7 Cj4gPiArfQo+ID4gKwo+ID4gK3N0YXRpYyBzc2l6ZV90IHZpcnRpb19wc3RvcmVfZG9fZXJhc2Uo VmlydElPUHN0b3JlICpzLAo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgc3RydWN0IHZpcnRpb19wc3RvcmVfcmVxICpyZXEpCj4gPiArewo+ID4gKyAgICBjaGFyIHBh dGhbUEFUSF9NQVhdOwo+ID4gKwo+ID4gKyAgICB2aXJ0aW9fcHN0b3JlX3RvX2ZpbGVuYW1lKHMs IHBhdGgsIHNpemVvZihwYXRoKSwgcmVxKTsKPiA+ICsKPiA+ICsgICAgcmV0dXJuIHVubGluayhw YXRoKTsKPiA+ICt9Cj4gPiArCj4gPiArc3RhdGljIHZvaWQgdmlydGlvX3BzdG9yZV9oYW5kbGVf aW8oVmlydElPRGV2aWNlICp2ZGV2LCBWaXJ0UXVldWUgKnZxKQo+ID4gK3sKPiA+ICsgICAgVmly dElPUHN0b3JlICpzID0gVklSVElPX1BTVE9SRSh2ZGV2KTsKPiA+ICsgICAgVmlydFF1ZXVlRWxl bWVudCAqZWxlbTsKPiA+ICsgICAgc3RydWN0IHZpcnRpb19wc3RvcmVfcmVxIHJlcTsKPiA+ICsg ICAgc3RydWN0IHZpcnRpb19wc3RvcmVfcmVzIHJlczsKPiA+ICsgICAgc3NpemVfdCBsZW4gPSAw Owo+ID4gKyAgICBpbnQgcmV0Owo+ID4gKwo+ID4gKyAgICBmb3IgKDs7KSB7Cj4gPiArICAgICAg ICBlbGVtID0gdmlydHF1ZXVlX3BvcCh2cSwgc2l6ZW9mKFZpcnRRdWV1ZUVsZW1lbnQpKTsKPiA+ ICsgICAgICAgIGlmICghZWxlbSkgewo+ID4gKyAgICAgICAgICAgIHJldHVybjsKPiA+ICsgICAg ICAgIH0KPiA+ICsKPiA+ICsgICAgICAgIGlmIChlbGVtLT5vdXRfbnVtIDwgMSB8fCBlbGVtLT5p bl9udW0gPCAxKSB7Cj4gPiArICAgICAgICAgICAgZXJyb3JfcmVwb3J0KCJyZXF1ZXN0IG9yIHJl c3BvbnNlIGJ1ZmZlciBpcyBtaXNzaW5nIik7Cj4gPiArICAgICAgICAgICAgZXhpdCgxKTsKPiA+ ICsgICAgICAgIH0KPiA+ICsKPiA+ICsgICAgICAgIGxlbiA9IGlvdl90b19idWYoZWxlbS0+b3V0 X3NnLCBlbGVtLT5vdXRfbnVtLCAwLCAmcmVxLCBzaXplb2YocmVxKSk7Cj4gPiArICAgICAgICBp ZiAobGVuICE9IChzc2l6ZV90KXNpemVvZihyZXEpKSB7Cj4gPiArICAgICAgICAgICAgZXJyb3Jf cmVwb3J0KCJpbnZhbGlkIHJlcXVlc3Qgc2l6ZTogJWxkIiwgKGxvbmcpbGVuKTsKPiA+ICsgICAg ICAgICAgICBleGl0KDEpOwo+ID4gKyAgICAgICAgfQo+ID4gKyAgICAgICAgcmVzLmNtZCAgPSBy ZXEuY21kOwo+ID4gKyAgICAgICAgcmVzLnR5cGUgPSByZXEudHlwZTsKPiA+ICsKPiA+ICsgICAg ICAgIHN3aXRjaCAobGUxNl90b19jcHUocmVxLmNtZCkpIHsKPiA+ICsgICAgICAgIGNhc2UgVklS VElPX1BTVE9SRV9DTURfT1BFTjoKPiA+ICsgICAgICAgICAgICByZXQgPSB2aXJ0aW9fcHN0b3Jl X2RvX29wZW4ocyk7Cj4gPiArICAgICAgICAgICAgYnJlYWs7Cj4gPiArICAgICAgICBjYXNlIFZJ UlRJT19QU1RPUkVfQ01EX1JFQUQ6Cj4gPiArICAgICAgICAgICAgcmV0ID0gdmlydGlvX3BzdG9y ZV9kb19yZWFkKHMsIGVsZW0tPmluX3NnLCBlbGVtLT5pbl9udW0sICZyZXMpOwo+ID4gKyAgICAg ICAgICAgIGlmIChyZXQgPiAwKSB7Cj4gPiArICAgICAgICAgICAgICAgIGxlbiA9IHJldDsKPiA+ ICsgICAgICAgICAgICAgICAgcmV0ID0gMDsKPiA+ICsgICAgICAgICAgICB9Cj4gPiArICAgICAg ICAgICAgYnJlYWs7Cj4gPiArICAgICAgICBjYXNlIFZJUlRJT19QU1RPUkVfQ01EX1dSSVRFOgo+ ID4gKyAgICAgICAgICAgIHJldCA9IHZpcnRpb19wc3RvcmVfZG9fd3JpdGUocywgZWxlbS0+b3V0 X3NnLCBlbGVtLT5vdXRfbnVtLCAmcmVxKTsKPiA+ICsgICAgICAgICAgICBicmVhazsKPiA+ICsg ICAgICAgIGNhc2UgVklSVElPX1BTVE9SRV9DTURfQ0xPU0U6Cj4gPiArICAgICAgICAgICAgcmV0 ID0gdmlydGlvX3BzdG9yZV9kb19jbG9zZShzKTsKPiA+ICsgICAgICAgICAgICBicmVhazsKPiA+ ICsgICAgICAgIGNhc2UgVklSVElPX1BTVE9SRV9DTURfRVJBU0U6Cj4gPiArICAgICAgICAgICAg cmV0ID0gdmlydGlvX3BzdG9yZV9kb19lcmFzZShzLCAmcmVxKTsKPiA+ICsgICAgICAgICAgICBi cmVhazsKPiA+ICsgICAgICAgIGRlZmF1bHQ6Cj4gPiArICAgICAgICAgICAgcmV0ID0gLTE7Cj4g PiArICAgICAgICAgICAgYnJlYWs7Cj4gPiArICAgICAgICB9Cj4gPiArCj4gPiArICAgICAgICBy ZXMucmV0ICA9IHJldDsKPiA+ICsKPiA+ICsgICAgICAgIGlvdl9mcm9tX2J1ZihlbGVtLT5pbl9z ZywgZWxlbS0+aW5fbnVtLCAwLCAmcmVzLCBzaXplb2YocmVzKSk7Cj4gPiArICAgICAgICB2aXJ0 cXVldWVfcHVzaCh2cSwgZWxlbSwgc2l6ZW9mKHJlcykgKyBsZW4pOwo+IAo+IHRoaXMgaXMgd3Jv bmcgLSBsZW4gc2hvdWxkIGJlICMgb2YgYnl0ZXMgd3JpdHRlbiBpbnRvIGd1ZXN0IG1lbW9yeS4K Cj8/PwoKQWxsIGNvbW1hbmQgZXhjZXB0IFJFQUQgb25seSB3cml0ZSB0aGUgdmlydGlvX3BzdG9y ZV9yZXQgc28gbGVuIGlzIDAuCkZvciBSRUFELCB2aXJ0aW9fcHN0b3JlX2RvX3JlYWQoKSByZXR1 cm5zIHRoZSBhY3R1YWwgYnl0ZXMgaXQgd3JvdGUKKG1pbnVzIHNpemVvZihyZXMpIG9mIGNvdXJz ZSksIHNvIEkgdGhpbmsgaXQncyBmaW5lLCBubz8KCkFueXdheSwgdGhhbmtzIGZvciB5b3VyIHJl dmlldyEKClRoYW5rcywKTmFtaHl1bmcKCgo+IAo+ID4gKwo+ID4gKyAgICAgICAgdmlydGlvX25v dGlmeSh2ZGV2LCB2cSk7Cj4gPiArICAgICAgICBnX2ZyZWUoZWxlbSk7Cj4gPiArCj4gPiArICAg ICAgICBpZiAocmV0IDwgMCkgewo+ID4gKyAgICAgICAgICAgIHJldHVybjsKPiA+ICsgICAgICAg IH0KPiA+ICsgICAgfQo+ID4gK30KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KVmlydHVhbGl6YXRpb24gbWFpbGluZyBsaXN0ClZpcnR1YWxpemF0aW9uQGxp c3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnCmh0dHBzOi8vbGlzdHMubGludXhmb3VuZGF0aW9uLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3ZpcnR1YWxpemF0aW9u From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1162938AbcG1FkG (ORCPT ); Thu, 28 Jul 2016 01:40:06 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:35555 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751361AbcG1Fj6 convert rfc822-to-8bit (ORCPT ); Thu, 28 Jul 2016 01:39:58 -0400 X-Original-SENDERIP: 156.147.1.127 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 165.244.98.76 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 10.177.227.17 X-Original-MAILFROM: namhyung@kernel.org Date: Thu, 28 Jul 2016 14:39:53 +0900 From: Namhyung Kim To: "Michael S. Tsirkin" CC: , , , LKML , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Anthony Liguori , Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Steven Rostedt , Ingo Molnar , Minchan Kim Subject: Re: [PATCH 6/7] qemu: Implement virtio-pstore device Message-ID: <20160728053953.GB4371@sejong> References: <1469632111-23260-1-git-send-email-namhyung@kernel.org> <1469632111-23260-7-git-send-email-namhyung@kernel.org> <20160728023254-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 In-Reply-To: <20160728023254-mutt-send-email-mst@kernel.org> User-Agent: Mutt/1.6.2 (2016-07-01) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB07/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/07/28 14:39:53, Serialize by Router on LGEKRMHUB07/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2016/07/28 14:39:53 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote: > On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote: > > Add virtio pstore device to allow kernel log files saved on the host. > > It will save the log files on the directory given by pstore device > > option. > > > > $ qemu-system-x86_64 -device virtio-pstore,directory=dir-xx ... > > > > (guest) # echo c > /proc/sysrq-trigger > > So if the point is handling system crashes, I suspect > virtio is the wrong protocol to use. ATM it's rather > elaborate for performance, it's likely not to work when > linux is crashing. I think you want something very very > simple that will still work when you crash. Like maybe > a serial device? Well, I dont' know. As you know, the kernel oops dump is already sent to serial device but it's rather slow. As I wrote in the cover letter, enabling ftrace_dump_on_oops makes it even worse.. Also pstore saves the (compressed) binary data so I thought it'd be better to have a dedicated IO channel. I know that we can't rely on anything in kernel when the kernel is crashing. In the virtualization environment, I think it'd be great if it can dump the crash data in the host directly so I chose the virtio. I thought the virtio is a relatively thin layer and independent from other kernel features. Even if it doesn't guarantee to work 100%, it's worth trying IMHO. > > > $ ls dir-xx > > dmesg-1.enc.z dmesg-2.enc.z > > > > The log files are usually compressed using zlib. Users can see the log > > messages directly on the host or on the guest (using pstore filesystem). > > So this lacks all management tools that a regular storage device > has, and these are necessary if people are to use this > in production. > > For example, some kind of provision for limiting the amount of host disk > this can consume seems called for. Rate-limiting disk writes > on host also seems necessary. Handling host disk errors always > propagates error to guest but an option to e.g. stop vm might > be useful to avoid corruption. Yes, it needs that kind of management. I'd like to look at the existing implementation. But basically I thought it as a debugging feature and it won't produce much data for the default setting. Maybe I can limit the file size not to exceed the buffer size and keep up to pre-configured number of files for each type. Now host disk error will be propagated to guest. > > > > > The 'directory' property is required for virtio-pstore device to work. > > It also adds 'bufsize' and 'console' (boolean) properties. > > No idea what these do. Seem to be RW by guest but have no effect > otherwise. The 'directory' is a pathname which it saves the data files. The 'bufsize' sets the size of buffer used by pstore. The 'console' enables saving pstore console type data. > > > > Cc: Paolo Bonzini > > Cc: Radim Krčmář > > Cc: "Michael S. Tsirkin" > > Cc: Anthony Liguori > > Cc: Anton Vorontsov > > Cc: Colin Cross > > Cc: Kees Cook > > Cc: Tony Luck > > Cc: Steven Rostedt > > Cc: Ingo Molnar > > Cc: Minchan Kim > > Cc: kvm@vger.kernel.org > > Cc: qemu-devel@nongnu.org > > Cc: virtualization@lists.linux-foundation.org > > Signed-off-by: Namhyung Kim > > --- [SNIP] > > diff --git a/hw/virtio/virtio-pstore.c b/hw/virtio/virtio-pstore.c > > new file mode 100644 > > index 0000000..2ca7786 > > --- /dev/null > > +++ b/hw/virtio/virtio-pstore.c > > @@ -0,0 +1,477 @@ > > +/* > > + * Virtio Pstore Device > > + * > > + * Copyright (C) 2016 LG Electronics > > + * > > + * Authors: > > + * Namhyung Kim > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2. > > We generally ask new code to be v2 or later. Ok, will update. > > > See > > + * the COPYING file in the top-level directory. > > + * > > + */ > > + > > +#include > > + > > +#include "qemu/osdep.h" > > +#include "qemu/iov.h" > > +#include "qemu-common.h" > > +#include "qemu/cutils.h" > > +#include "qemu/error-report.h" > > +#include "sysemu/kvm.h" > > +#include "qapi/visitor.h" > > +#include "qapi-event.h" > > +#include "trace.h" > > + > > +#include "hw/virtio/virtio.h" > > +#include "hw/virtio/virtio-bus.h" > > +#include "hw/virtio/virtio-access.h" > > +#include "hw/virtio/virtio-pstore.h" > > + > > + > > +static void virtio_pstore_to_filename(VirtIOPstore *s, char *buf, size_t sz, > > + struct virtio_pstore_req *req) > > +{ > > + const char *basename; > > + unsigned long long id = 0; > > + unsigned int flags = le32_to_cpu(req->flags); > > + > > + switch (le16_to_cpu(req->type)) { > > + case VIRTIO_PSTORE_TYPE_DMESG: > > + basename = "dmesg"; > > + id = s->id++; > > + break; > > + case VIRTIO_PSTORE_TYPE_CONSOLE: > > + basename = "console"; > > + if (s->console_id) { > > + id = s->console_id; > > + } else { > > + id = s->console_id = s->id++; > > + } > > + break; > > + default: > > + basename = "unknown"; > > + break; > > + } > > + > > + snprintf(buf, sz, "%s/%s-%llu%s", s->directory, basename, id, > > + flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" : ""); > > +} > > + > > +static void virtio_pstore_from_filename(VirtIOPstore *s, char *name, > > + char *buf, size_t sz, > > + struct virtio_pstore_fileinfo *info) > > +{ > > + snprintf(buf, sz, "%s/%s", s->directory, name); > > if this does not fit, buf will not be 0 terminated and can > generally be corrupted. Will change it to use malloc instead. > > > > + > > + if (g_str_has_prefix(name, "dmesg-")) { > > + info->type = VIRTIO_PSTORE_TYPE_DMESG; > > + name += strlen("dmesg-"); > > + } else if (g_str_has_prefix(name, "console-")) { > > + info->type = VIRTIO_PSTORE_TYPE_CONSOLE; > > + name += strlen("console-"); > > + } else if (g_str_has_prefix(name, "unknown-")) { > > + info->type = VIRTIO_PSTORE_TYPE_UNKNOWN; > > + name += strlen("unknown-"); > > + } > > + > > + qemu_strtoull(name, NULL, 0, &info->id); > > + > > + info->flags = 0; > > + if (g_str_has_suffix(name, ".enc.z")) { > > + info->flags |= VIRTIO_PSTORE_FL_COMPRESSED; > > + } > > +} [SNIP] > > +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, struct iovec *out_sg, > > + unsigned int out_num, > > + struct virtio_pstore_req *req) > > +{ > > + char path[PATH_MAX]; > > + int fd; > > + ssize_t len; > > + unsigned short type; > > + int flags = O_WRONLY | O_CREAT; > > + > > + /* we already consume the req */ > > + iov_discard_front(&out_sg, &out_num, sizeof(*req)); > > + > > + virtio_pstore_to_filename(s, path, sizeof(path), req); > > + > > + type = le16_to_cpu(req->type); > > + > > + if (type == VIRTIO_PSTORE_TYPE_DMESG) { > > + flags |= O_TRUNC; > > + } else if (type == VIRTIO_PSTORE_TYPE_CONSOLE) { > > + flags |= O_APPEND; > > + } > > + > > + fd = open(path, flags, 0644); > > + if (fd < 0) { > > + error_report("cannot open %s", path); > > + return -1; > > + } > > + len = writev(fd, out_sg, out_num); > > + close(fd); > > + > > + return len; > > All this is blocking VM until host io completes. Hmm.. I don't know about the internals of qemu. So does it make guest stop? If so, that's what I want to do for _DMESG. :) As it's called only on kernel oops I think it's admittable. But for _CONSOLE, it needs to do asynchronously. Maybe I can add a thread to do the work. > > > > +} > > + > > +static ssize_t virtio_pstore_do_close(VirtIOPstore *s) > > +{ > > + if (s->dirp == NULL) { > > + return 0; > > + } > > + > > + closedir(s->dirp); > > + s->dirp = NULL; > > + > > + return 0; > > +} > > + > > +static ssize_t virtio_pstore_do_erase(VirtIOPstore *s, > > + struct virtio_pstore_req *req) > > +{ > > + char path[PATH_MAX]; > > + > > + virtio_pstore_to_filename(s, path, sizeof(path), req); > > + > > + return unlink(path); > > +} > > + > > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq) > > +{ > > + VirtIOPstore *s = VIRTIO_PSTORE(vdev); > > + VirtQueueElement *elem; > > + struct virtio_pstore_req req; > > + struct virtio_pstore_res res; > > + ssize_t len = 0; > > + int ret; > > + > > + for (;;) { > > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > > + if (!elem) { > > + return; > > + } > > + > > + if (elem->out_num < 1 || elem->in_num < 1) { > > + error_report("request or response buffer is missing"); > > + exit(1); > > + } > > + > > + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req)); > > + if (len != (ssize_t)sizeof(req)) { > > + error_report("invalid request size: %ld", (long)len); > > + exit(1); > > + } > > + res.cmd = req.cmd; > > + res.type = req.type; > > + > > + switch (le16_to_cpu(req.cmd)) { > > + case VIRTIO_PSTORE_CMD_OPEN: > > + ret = virtio_pstore_do_open(s); > > + break; > > + case VIRTIO_PSTORE_CMD_READ: > > + ret = virtio_pstore_do_read(s, elem->in_sg, elem->in_num, &res); > > + if (ret > 0) { > > + len = ret; > > + ret = 0; > > + } > > + break; > > + case VIRTIO_PSTORE_CMD_WRITE: > > + ret = virtio_pstore_do_write(s, elem->out_sg, elem->out_num, &req); > > + break; > > + case VIRTIO_PSTORE_CMD_CLOSE: > > + ret = virtio_pstore_do_close(s); > > + break; > > + case VIRTIO_PSTORE_CMD_ERASE: > > + ret = virtio_pstore_do_erase(s, &req); > > + break; > > + default: > > + ret = -1; > > + break; > > + } > > + > > + res.ret = ret; > > + > > + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res)); > > + virtqueue_push(vq, elem, sizeof(res) + len); > > this is wrong - len should be # of bytes written into guest memory. ??? All command except READ only write the virtio_pstore_ret so len is 0. For READ, virtio_pstore_do_read() returns the actual bytes it wrote (minus sizeof(res) of course), so I think it's fine, no? Anyway, thanks for your review! Thanks, Namhyung > > > + > > + virtio_notify(vdev, vq); > > + g_free(elem); > > + > > + if (ret < 0) { > > + return; > > + } > > + } > > +} From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41080) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSe3A-0002N6-HQ for qemu-devel@nongnu.org; Thu, 28 Jul 2016 01:40:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSe36-0001Gc-7L for qemu-devel@nongnu.org; Thu, 28 Jul 2016 01:40:03 -0400 Received: from lgeamrelo13.lge.com ([156.147.23.53]:36784) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSe35-0001GR-7i for qemu-devel@nongnu.org; Thu, 28 Jul 2016 01:40:00 -0400 Date: Thu, 28 Jul 2016 14:39:53 +0900 From: Namhyung Kim Message-ID: <20160728053953.GB4371@sejong> References: <1469632111-23260-1-git-send-email-namhyung@kernel.org> <1469632111-23260-7-git-send-email-namhyung@kernel.org> <20160728023254-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 In-Reply-To: <20160728023254-mutt-send-email-mst@kernel.org> Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, LKML , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Anthony Liguori , Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Steven Rostedt , Ingo Molnar , Minchan Kim On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote: > On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote: > > Add virtio pstore device to allow kernel log files saved on the host. > > It will save the log files on the directory given by pstore device > > option. > >=20 > > $ qemu-system-x86=5F64 -device virtio-pstore,directory=3Ddir-xx ... > >=20 > > (guest) # echo c > /proc/sysrq-trigger >=20 > So if the point is handling system crashes, I suspect > virtio is the wrong protocol to use. ATM it's rather > elaborate for performance, it's likely not to work when > linux is crashing. I think you want something very very > simple that will still work when you crash. Like maybe > a serial device? Well, I dont' know. As you know, the kernel oops dump is already sent to serial device but it's rather slow. As I wrote in the cover letter, enabling ftrace=5Fdump=5Fon=5Foops makes it even worse.. Also pstore saves the (compressed) binary data so I thought it'd be better to have a dedicated IO channel. I know that we can't rely on anything in kernel when the kernel is crashing. In the virtualization environment, I think it'd be great if it can dump the crash data in the host directly so I chose the virtio. I thought the virtio is a relatively thin layer and independent from other kernel features. Even if it doesn't guarantee to work 100%, it's worth trying IMHO. >=20 > > $ ls dir-xx > > dmesg-1.enc.z dmesg-2.enc.z > >=20 > > The log files are usually compressed using zlib. Users can see the log > > messages directly on the host or on the guest (using pstore filesystem). >=20 > So this lacks all management tools that a regular storage device > has, and these are necessary if people are to use this > in production. >=20 > For example, some kind of provision for limiting the amount of host disk > this can consume seems called for. Rate-limiting disk writes > on host also seems necessary. Handling host disk errors always > propagates error to guest but an option to e.g. stop vm might > be useful to avoid corruption. Yes, it needs that kind of management. I'd like to look at the existing implementation. But basically I thought it as a debugging feature and it won't produce much data for the default setting. Maybe I can limit the file size not to exceed the buffer size and keep up to pre-configured number of files for each type. Now host disk error will be propagated to guest. >=20 > >=20 > > The 'directory' property is required for virtio-pstore device to work. > > It also adds 'bufsize' and 'console' (boolean) properties. >=20 > No idea what these do. Seem to be RW by guest but have no effect > otherwise. The 'directory' is a pathname which it saves the data files. The 'bufsize' sets the size of buffer used by pstore. The 'console' enables saving pstore console type data. >=20 >=20 > > Cc: Paolo Bonzini > > Cc: Radim Kr=C4=8Dm=C3=A1=C5=99 > > Cc: "Michael S. Tsirkin" > > Cc: Anthony Liguori > > Cc: Anton Vorontsov > > Cc: Colin Cross > > Cc: Kees Cook > > Cc: Tony Luck > > Cc: Steven Rostedt > > Cc: Ingo Molnar > > Cc: Minchan Kim > > Cc: kvm@vger.kernel.org > > Cc: qemu-devel@nongnu.org > > Cc: virtualization@lists.linux-foundation.org > > Signed-off-by: Namhyung Kim > > --- [SNIP] > > diff --git a/hw/virtio/virtio-pstore.c b/hw/virtio/virtio-pstore.c > > new file mode 100644 > > index 0000000..2ca7786 > > --- /dev/null > > +++ b/hw/virtio/virtio-pstore.c > > @@ -0,0 +1,477 @@ > > +/* > > + * Virtio Pstore Device > > + * > > + * Copyright (C) 2016 LG Electronics > > + * > > + * Authors: > > + * Namhyung Kim > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2. >=20 > We generally ask new code to be v2 or later. Ok, will update. >=20 > > See > > + * the COPYING file in the top-level directory. > > + * > > + */ > > + > > +#include > > + > > +#include "qemu/osdep.h" > > +#include "qemu/iov.h" > > +#include "qemu-common.h" > > +#include "qemu/cutils.h" > > +#include "qemu/error-report.h" > > +#include "sysemu/kvm.h" > > +#include "qapi/visitor.h" > > +#include "qapi-event.h" > > +#include "trace.h" > > + > > +#include "hw/virtio/virtio.h" > > +#include "hw/virtio/virtio-bus.h" > > +#include "hw/virtio/virtio-access.h" > > +#include "hw/virtio/virtio-pstore.h" > > + > > + > > +static void virtio=5Fpstore=5Fto=5Ffilename(VirtIOPstore *s, char *buf= , size=5Ft sz, > > + struct virtio=5Fpstore=5Freq *re= q) > > +{ > > + const char *basename; > > + unsigned long long id =3D 0; > > + unsigned int flags =3D le32=5Fto=5Fcpu(req->flags); > > + > > + switch (le16=5Fto=5Fcpu(req->type)) { > > + case VIRTIO=5FPSTORE=5FTYPE=5FDMESG: > > + basename =3D "dmesg"; > > + id =3D s->id++; > > + break; > > + case VIRTIO=5FPSTORE=5FTYPE=5FCONSOLE: > > + basename =3D "console"; > > + if (s->console=5Fid) { > > + id =3D s->console=5Fid; > > + } else { > > + id =3D s->console=5Fid =3D s->id++; > > + } > > + break; > > + default: > > + basename =3D "unknown"; > > + break; > > + } > > + > > + snprintf(buf, sz, "%s/%s-%llu%s", s->directory, basename, id, > > + flags & VIRTIO=5FPSTORE=5FFL=5FCOMPRESSED ? ".enc.z" : ""= ); > > +} > > + > > +static void virtio=5Fpstore=5Ffrom=5Ffilename(VirtIOPstore *s, char *n= ame, > > + char *buf, size=5Ft sz, > > + struct virtio=5Fpstore=5Ffilei= nfo *info) > > +{ > > + snprintf(buf, sz, "%s/%s", s->directory, name); >=20 > if this does not fit, buf will not be 0 terminated and can > generally be corrupted. Will change it to use malloc instead. >=20 >=20 > > + > > + if (g=5Fstr=5Fhas=5Fprefix(name, "dmesg-")) { > > + info->type =3D VIRTIO=5FPSTORE=5FTYPE=5FDMESG; > > + name +=3D strlen("dmesg-"); > > + } else if (g=5Fstr=5Fhas=5Fprefix(name, "console-")) { > > + info->type =3D VIRTIO=5FPSTORE=5FTYPE=5FCONSOLE; > > + name +=3D strlen("console-"); > > + } else if (g=5Fstr=5Fhas=5Fprefix(name, "unknown-")) { > > + info->type =3D VIRTIO=5FPSTORE=5FTYPE=5FUNKNOWN; > > + name +=3D strlen("unknown-"); > > + } > > + > > + qemu=5Fstrtoull(name, NULL, 0, &info->id); > > + > > + info->flags =3D 0; > > + if (g=5Fstr=5Fhas=5Fsuffix(name, ".enc.z")) { > > + info->flags |=3D VIRTIO=5FPSTORE=5FFL=5FCOMPRESSED; > > + } > > +} [SNIP] > > +static ssize=5Ft virtio=5Fpstore=5Fdo=5Fwrite(VirtIOPstore *s, struct = iovec *out=5Fsg, > > + unsigned int out=5Fnum, > > + struct virtio=5Fpstore=5Freq *re= q) > > +{ > > + char path[PATH=5FMAX]; > > + int fd; > > + ssize=5Ft len; > > + unsigned short type; > > + int flags =3D O=5FWRONLY | O=5FCREAT; > > + > > + /* we already consume the req */ > > + iov=5Fdiscard=5Ffront(&out=5Fsg, &out=5Fnum, sizeof(*req)); > > + > > + virtio=5Fpstore=5Fto=5Ffilename(s, path, sizeof(path), req); > > + > > + type =3D le16=5Fto=5Fcpu(req->type); > > + > > + if (type =3D=3D VIRTIO=5FPSTORE=5FTYPE=5FDMESG) { > > + flags |=3D O=5FTRUNC; > > + } else if (type =3D=3D VIRTIO=5FPSTORE=5FTYPE=5FCONSOLE) { > > + flags |=3D O=5FAPPEND; > > + } > > + > > + fd =3D open(path, flags, 0644); > > + if (fd < 0) { > > + error=5Freport("cannot open %s", path); > > + return -1; > > + } > > + len =3D writev(fd, out=5Fsg, out=5Fnum); > > + close(fd); > > + > > + return len; >=20 > All this is blocking VM until host io completes. Hmm.. I don't know about the internals of qemu. So does it make guest stop? If so, that's what I want to do for =5FDMESG. :) As it's called only on kernel oops I think it's admittable. But for =5FCONSOLE, it needs to do asynchronously. Maybe I can add a thread to do the work. >=20 >=20 > > +} > > + > > +static ssize=5Ft virtio=5Fpstore=5Fdo=5Fclose(VirtIOPstore *s) > > +{ > > + if (s->dirp =3D=3D NULL) { > > + return 0; > > + } > > + > > + closedir(s->dirp); > > + s->dirp =3D NULL; > > + > > + return 0; > > +} > > + > > +static ssize=5Ft virtio=5Fpstore=5Fdo=5Ferase(VirtIOPstore *s, > > + struct virtio=5Fpstore=5Freq *re= q) > > +{ > > + char path[PATH=5FMAX]; > > + > > + virtio=5Fpstore=5Fto=5Ffilename(s, path, sizeof(path), req); > > + > > + return unlink(path); > > +} > > + > > +static void virtio=5Fpstore=5Fhandle=5Fio(VirtIODevice *vdev, VirtQueu= e *vq) > > +{ > > + VirtIOPstore *s =3D VIRTIO=5FPSTORE(vdev); > > + VirtQueueElement *elem; > > + struct virtio=5Fpstore=5Freq req; > > + struct virtio=5Fpstore=5Fres res; > > + ssize=5Ft len =3D 0; > > + int ret; > > + > > + for (;;) { > > + elem =3D virtqueue=5Fpop(vq, sizeof(VirtQueueElement)); > > + if (!elem) { > > + return; > > + } > > + > > + if (elem->out=5Fnum < 1 || elem->in=5Fnum < 1) { > > + error=5Freport("request or response buffer is missing"); > > + exit(1); > > + } > > + > > + len =3D iov=5Fto=5Fbuf(elem->out=5Fsg, elem->out=5Fnum, 0, &re= q, sizeof(req)); > > + if (len !=3D (ssize=5Ft)sizeof(req)) { > > + error=5Freport("invalid request size: %ld", (long)len); > > + exit(1); > > + } > > + res.cmd =3D req.cmd; > > + res.type =3D req.type; > > + > > + switch (le16=5Fto=5Fcpu(req.cmd)) { > > + case VIRTIO=5FPSTORE=5FCMD=5FOPEN: > > + ret =3D virtio=5Fpstore=5Fdo=5Fopen(s); > > + break; > > + case VIRTIO=5FPSTORE=5FCMD=5FREAD: > > + ret =3D virtio=5Fpstore=5Fdo=5Fread(s, elem->in=5Fsg, elem= ->in=5Fnum, &res); > > + if (ret > 0) { > > + len =3D ret; > > + ret =3D 0; > > + } > > + break; > > + case VIRTIO=5FPSTORE=5FCMD=5FWRITE: > > + ret =3D virtio=5Fpstore=5Fdo=5Fwrite(s, elem->out=5Fsg, el= em->out=5Fnum, &req); > > + break; > > + case VIRTIO=5FPSTORE=5FCMD=5FCLOSE: > > + ret =3D virtio=5Fpstore=5Fdo=5Fclose(s); > > + break; > > + case VIRTIO=5FPSTORE=5FCMD=5FERASE: > > + ret =3D virtio=5Fpstore=5Fdo=5Ferase(s, &req); > > + break; > > + default: > > + ret =3D -1; > > + break; > > + } > > + > > + res.ret =3D ret; > > + > > + iov=5Ffrom=5Fbuf(elem->in=5Fsg, elem->in=5Fnum, 0, &res, sizeo= f(res)); > > + virtqueue=5Fpush(vq, elem, sizeof(res) + len); >=20 > this is wrong - len should be # of bytes written into guest memory. ??? All command except READ only write the virtio=5Fpstore=5Fret so len is 0. For READ, virtio=5Fpstore=5Fdo=5Fread() returns the actual bytes it wrote (minus sizeof(res) of course), so I think it's fine, no? Anyway, thanks for your review! Thanks, Namhyung >=20 > > + > > + virtio=5Fnotify(vdev, vq); > > + g=5Ffree(elem); > > + > > + if (ret < 0) { > > + return; > > + } > > + } > > +}