From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Wolf Subject: Re: [PATCH v4 16/18] xen: automatically create XenBlockDevice-s Date: Thu, 13 Dec 2018 16:08:18 +0100 Message-ID: <20181213150818.GI5427@linux.fritz.box> References: <1544543862-9997-1-git-send-email-paul.durrant@citrix.com> <1544543862-9997-17-git-send-email-paul.durrant@citrix.com> <20181213115152.GA5427@linux.fritz.box> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gXSbA-0000lh-KU for xen-devel@lists.xenproject.org; Thu, 13 Dec 2018 15:08:24 +0000 Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Paul Durrant Cc: "xen-devel@lists.xenproject.org" , Stefano Stabellini , "qemu-devel@nongnu.org" , "qemu-block@nongnu.org" , Max Reitz List-Id: xen-devel@lists.xenproject.org QW0gMTMuMTIuMjAxOCB1bSAxMzo0NCBoYXQgUGF1bCBEdXJyYW50IGdlc2NocmllYmVuOgo+ID4g RXNzZW50aWFsbHksIHdoYXQgSSdtIHdvbmRlcmluZyBpcyB3aGV0aGVyIHdlIGhhdmUgYW55dGhp bmcgdGhhdCBjb3VsZAo+ID4gYmUgdHJlYXRlZCBtb3JlIG9yIGxlc3MgbGlrZSBhbm90aGVyIG1v bml0b3IgYmVzaWRlcyBRTVAgYW5kIEhNUCwgd2hpY2gKPiA+IHdvdWxkIGludGVybmFsbHkgd29y ayBzaW1pbGFyIHRvIEhNUCwgaS5lLiBtYXAgKGFsbW9zdCkgZXZlcnl0aGluZyB0bwo+ID4gUU1Q IGNvbW1hbmRzLgo+IAo+IFllcywgaXQgd291bGQgYmUgcG9zc2libGUgdG8gaGF2ZSBhIHNlcGFy YXRlICdjb21wYXRpYmlsaXR5JyBkYWVtb24gdG8KPiB3YXRjaCB4ZW5zdG9yZSBhbmQgdGhlbiBm b3JtdWxhdGUgdGhlIGNvcnJlY3Qgc2VxdWVuY2Ugb2YgUU1QIGNvbW1hbmRzCj4gdG8gaW5zdGFu dGlhdGUgdGhlIGJhY2tlbmQsIGJ1dCB0aGF0IGlzIG1vcmUgY29tcGxpY2F0ZWQgYW5kIHRoZSBy aWdodAo+IGFuc3dlciBvZiBjb3Vyc2UgaXMgdG8gaGF2ZSB0aGUgdG9vbHN0YWNrIHNlbmQgdGhl IFFNUCBjb21tYW5kcyBpbiB0aGUKPiBmaXJzdCBwbGFjZS4KCk9rYXksIGlmIHNvbWVvbmUgaXMg d29ya2luZyBvbiBhY3R1YWxseSB1c2luZyBRTVAgaW5zdGVhZCBvZiB4ZW5zdG9yZSwKdGhhdCdz IGV2ZW4gYmV0dGVyLCBvZiBjb3Vyc2UuIERpc3JlZ2FyZCB0aGlzIHBvaW50IHRoZW4uCgo+ID4g PiArICAgIGRyaXZlX25ldyhkcml2ZV9vcHRzLCBJRl9OT05FLCAmbG9jYWxfZXJyKTsKPiA+ID4g KyAgICBpZiAobG9jYWxfZXJyKSB7Cj4gPiA+ICsgICAgICAgIGVycm9yX3Byb3BhZ2F0ZV9wcmVw ZW5kKGVycnAsIGxvY2FsX2VyciwKPiA+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgImZhaWxlZCB0byBjcmVhdGUgZHJpdmU6ICIpOwo+ID4gPiArICAgICAgICBnb3RvIGRvbmU7 Cj4gPiA+ICsgICAgfQo+ID4gCj4gPiBUaGUgb3RoZXIgbWFqb3IgcG9pbnQgaXMgdGhhdCB5b3Un cmUgdXNpbmcgdGhlIGxlZ2FjeSBkcml2ZV8qKCkKPiA+IGluZnJhc3RydWN0dXJlLCB3aGljaCBz aG91bGQgbm90IG9ubHkgZ28gYXdheSBhcyBzb29uIGFzIHdlIGNhbiwgYnV0Cj4gPiB3aGljaCBp cyBhbHNvIGZ1bGwgb2YgbWFnaWMgYW5kIG5hc3R5IHN1cnByaXNlcy4KPiA+IAo+ID4gSSB0aGlu ayB0aGUgYmVzdCB3YXkgd291bGQgYmUgdG8gY3JlYXRlIG9ubHkgYSBibG9jayBub2RlCj4gPiAo QmxvY2tEcml2ZXJTdGF0ZSkgaGVyZSwgYW5kIGdldCBhbiBhdXRvbWF0aWNhbGx5IGNyZWF0ZWQg YW5vbnltb3VzCj4gPiBCbG9ja0JhY2tlbmQgZnJvbSB0aGUgcWRldiBkcml2ZSBwcm9wZXJ0eS4K PiA+IAo+ID4gVGhlcmUgYXJlIHR3byB3YXlzIHRvIGFjaGlldmUgdGhpczogcW1wX2Jsb2NrZGV2 X2FkZCgpIHdvdWxkIGJlIG9wdGltYWwKPiA+IGJlY2F1c2UgdGhhdCdzIGEgc3RhYmxlIGV4dGVy bmFsIGludGVyZmFjZS4gSXQgd291bGQgcmVxdWlyZSB5b3UgdG8KPiA+IHNwZWNpZnkgYSBub2Rl LW5hbWUgKHlvdSBhbHJlYWR5IGhhdmUgdGhlIGlkIHBhcmFtZXRlciksIGFuZCB5b3UnZCB1c2UK PiA+IHRoaXMgbm9kZS1uYW1lIGZvciB0aGUgcWRldiBkcml2ZSBwcm9wZXJ0eS4KPiA+IAo+ID4g cW1wX2Jsb2NrZGV2X2FkZCgpIHJlcXVpcmVzIGEgQmxvY2tkZXZPcHRpb25zIG9iamVjdCwgd2hp Y2ggeW91IGNhbgo+ID4gZWl0aGVyIGNvbnN0cnVjdCBtYW51YWxseSBpbiBDIG9yIHVzZSBhIHZp c2l0b3IgdG8gY29udmVydCBmcm9tIGFuCj4gPiBvcHRpb25zIFFEaWN0LiBNYXliZSBpbiB0aGlz IGNhc2UsIGNvbnZlcnRpbmcgZnJvbSBhIFFEaWN0IGlzIGJldHRlcgo+ID4gYmVjYXVzZSBvdGhl cndpc2UgeW91IG5lZWQgc3BlY2lhbCBjb2RlIGZvciBlYWNoIGJsb2NrIGRyaXZlci4KPiA+IAo+ IAo+IEkgd2FzIHVzaW5nIHRoZSBsZWdhY3kgaW50ZXJmYWNlcyBiZWNhdXNlIHRoaXMgY29kZSBp cywgYXMgSSBzYWlkCj4gYWJvdmUsIHN1cHBvc2VkIHRvIGJlIGEgbWVjaGFuaXNtIG9ubHkgcmVx dWlyZWQgZm9yIGNvbXBhdGliaWxpdHkgd2l0aAo+IHRoZSB3YXkgdG9vbHN0YWNrcyBjdXJyZW50 bHkgb3BlcmF0ZSAoYW5kIHNvIGlzIGVzc2VudGlhbGx5ICdsZWdhY3knKQo+IGJ1dCB1c2luZyB0 aGUgdG9wLWxldmVsIFFNUCBlbnRyeSBwb2ludCB0byBjb25zdHJ1Y3QgdGhlIGRvZXMgc291bmQK PiBkby1hYmxlIGFzIGxvbmcgYXMgdGhlIHVuZGVybHlpbmcgZmlsZSBsb2NraW5nIGNhbiBzdGls bCBiZSBhdm9pZGVkCj4gd2l0aCB0aGF0IG1lY2hhbmlzbS4gU2luY2UgQmxvY2tkZXZPcHRpb25z IHNlZW1zIHRvIGJlIGFuCj4gYXV0by1nZW5lcmF0ZWQgc3RydWN0dXJlLCBmaWd1cmluZyBvdXQg aG93IHRvIGZpbGwgaXQgaW4gbWFudWFsbHkgaXMKPiBzb21ld2hhdCB0cmlja3kgc28gdGhlIFFE aWN0IGFwcHJvYWNoIGlzIHByZWZlcmFibGUgYnV0IEknbGwgaGF2ZSB0bwo+IGZpZ3VyZSBvdXQg aG93IHRvIHVzZSBhIHZpc2l0b3IgdG8gZG8gdGhlIHRyYW5zbGF0aW9uLgoKWW91IGNhbiBncmVw IGZvciBxb2JqZWN0X2lucHV0X3Zpc2l0b3JfbmV3KCkgZm9yIGV4YW1wbGVzLiBTb21lIGJsb2Nr CmRyaXZlcnMgdXNlIHRoaXMgaW50ZXJuYWxseSB0byBjb252ZXJ0IC5iZHJ2X2NyZWF0ZSBvcHRp b25zIHRvIGEgUUFQSQpvYmplY3QsIGl0IGxvb2tzIGxpa2UgdGhpczoKCiAgICAvKiBOb3cgZ2V0 IHRoZSBRQVBJIHR5cGUgQmxvY2tkZXZDcmVhdGVPcHRpb25zICovCiAgICB2ID0gcW9iamVjdF9p bnB1dF92aXNpdG9yX25ld19mbGF0X2NvbmZ1c2VkKHFkaWN0LCBlcnJwKTsKICAgIGlmICghdikg ewogICAgICAgIHJldCA9IC1FSU5WQUw7CiAgICAgICAgZ290byBmaW5pc2g7CiAgICB9CgogICAg dmlzaXRfdHlwZV9CbG9ja2RldkNyZWF0ZU9wdGlvbnModiwgTlVMTCwgJmNyZWF0ZV9vcHRpb25z LCAmbG9jYWxfZXJyKTsKICAgIHZpc2l0X2ZyZWUodik7CgogICAgaWYgKGxvY2FsX2Vycikgewog ICAgICAgIGVycm9yX3Byb3BhZ2F0ZShlcnJwLCBsb2NhbF9lcnIpOwogICAgICAgIHJldCA9IC1F SU5WQUw7CiAgICAgICAgZ290byBmaW5pc2g7CiAgICB9CgpPYnZpb3VzbHksIHlvdSdkIHVzZSB2 aXNpdF90eXBlX0Jsb2NrZGV2T3B0aW9ucyBpbnN0ZWFkLiBZb3UgYWxzbyBtaWdodApub3QgbmVl ZCB0aGUgX2ZsYXRfY29uZnVzZWQgdmFyaWFudCwgd2hpY2ggaXMgYWJvdXQgUURpY3RzIHdoZXJl IHdlCmRvbid0IGtub3cgd2hldGhlciB2YWx1ZXMgYXJlIHN0b3JlZCBhcyB0aGVpciBhY3R1YWwg dHlwZSBvciBhcyBzdHJpbmdzLgpJbiB5b3VyIGNvZGUsIHlvdSBoYXZlIGNvbnRyb2wgb3ZlciB0 aGUgdHlwZXMgaW4gdGhlIFFEaWN0LCBzbyB0aGlzCnNob3VsZG4ndCBiZSBhIHByb2JsZW0uCgo+ ID4gVGhlIG90aGVyIHdheSB3b3VsZCBiZSBjYWxsaW5nIGJkcnZfb3BlbigpIGRpcmVjdGx5LCB3 aGljaCBnaXZlcyB5b3UgYQo+ID4gQmxvY2tEcml2ZXJTdGF0ZSwgYnV0IGl0IHJpc2tzIHVzaW5n IGxlZ2FjeSBmdW5jdGlvbmFsaXR5IHRoYXQgd2lsbCBiZQo+ID4gZGVwcmVjYXRlZCBzb29uLiBB Z2FpbiwgeW91J2QgdGFrZSB0aGUgbm9kZS1uYW1lIGFuZCBwYXNzIGl0IHRvIHRoZSBxZGV2Cj4g PiBkcml2ZSBvcHRpb24gYmVsb3cuCj4gCj4gWWVzLCB4ZW5fZGlzayBkb2VzIHRoaW5ncyB0aGlz IHdheSBidXQgdGhlbiB3ZSBlbmQgdXAgd2l0aCBsZWdhY3kKPiBibG9jayBkZXZpY2UgYW5kIHN0 aWxsIGZhbGwgZm91bCBvZiB0aGUgYXNzZXJ0aW9ucyBidXJpZWQgaW4gdGhlIGNvZGUuCgpZZXMg YW5kIG5vLiB4ZW5fZGlzayBpcyBiZXR0ZXIgaW4gdGhhdCBpdCBhdm9pZHMgdGhlIGRyaXZlXyog dGhpbmdzCih3aGljaCBpbnRlcm5hbGx5IGNhbGwgYmRydl9vcGVuKCkgYW55d2F5KSwgYnV0IGl0 J3Mgd29yc2UgaW4gdGhhdCBpdApkaXJlY3RseSBhc3NpZ25zIGJsa2Rldi0+YmxrIGluc3RlYWQg b2YgdXNpbmcgdGhlIHFkZXYgcHJvcGVydHkuCgpXaGF0IEkgbWVhbnQgaGVyZSBpcyB0aGF0IHlv dSBjcmVhdGUgdGhlIEJEUyB3aXRoIGJkcnZfb3BlbigpLCBidXQgdGhlbgp5b3Ugd291bGRuJ3Qg YXNzaWduIGl0IGRpcmVjdGx5IHRvIHNvbWUgZmllbGQgaW4gdGhlIGRldmljZSBzdGF0ZSwgYnV0 Cmp1c3QgcHV0IHRoZSBub2RlLW5hbWUgb2YgdGhlIEJEUyBpbnRvIHRoZSBxZGV2IHByb3BlcnR5 ICdkcml2ZScuIFRoaXMKd291bGQgYmUgYWxtb3N0IGxpa2UgcW1wX2Jsb2NrZGV2X2FkZCgpLCBl eGNlcHQgd2l0aG91dCB2YWxpZGF0aW9uCmFnYWluc3QgdGhlIFFBUEkgc2NoZW1hLgoKQnV0IGlm IHVzaW5nIHFtcF9ibG9ja2Rldl9hZGQoKSBpcyBlYXN5IGVub3VnaCwgdGhhdCdzIHByZWZlcmFi bGUuCgo+ID4gCj4gPiA+ICsKPiA+ID4gK2RvbmU6Cj4gPiA+ICsgICAgZ19mcmVlKGRyaXZlX29w dHN0cik7Cj4gPiA+ICsgICAgZ19mcmVlKGZvcm1hdCk7Cj4gPiA+ICsgICAgZ19mcmVlKGZpbGUp Owo+ID4gPiArfQo+ID4gPiArCj4gPiA+ICtzdGF0aWMgdm9pZCB4ZW5fYmxvY2tfZGV2aWNlX2Ny ZWF0ZShCdXNTdGF0ZSAqYnVzLCBjb25zdCBjaGFyICpuYW1lLAo+ID4gPiArICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgUURpY3QgKm9wdHMsIEVycm9yICoqZXJycCkKPiA+ID4g K3sKPiA+ID4gKyAgICB1bnNpZ25lZCBsb25nIG51bWJlcjsKPiA+ID4gKyAgICBjb25zdCBjaGFy ICp2ZGV2LCAqZGV2aWNlX3R5cGU7Cj4gPiA+ICsgICAgQmxvY2tCYWNrZW5kICpibGsgPSBOVUxM Owo+ID4gPiArICAgIElPVGhyZWFkICppb3RocmVhZCA9IE5VTEw7Cj4gPiA+ICsgICAgRGV2aWNl U3RhdGUgKmRldiA9IE5VTEw7Cj4gPiA+ICsgICAgRXJyb3IgKmxvY2FsX2VyciA9IE5VTEw7Cj4g PiA+ICsgICAgY29uc3QgY2hhciAqdHlwZTsKPiA+ID4gKyAgICBYZW5CbG9ja0RldmljZSAqYmxv Y2tkZXY7Cj4gPiA+ICsKPiA+ID4gKyAgICB0cmFjZV94ZW5fYmxvY2tfZGV2aWNlX2NyZWF0ZShu YW1lKTsKPiA+ID4gKwo+ID4gPiArICAgIGlmIChxZW11X3N0cnRvdWwobmFtZSwgTlVMTCwgMTAs ICZudW1iZXIpKSB7Cj4gPiA+ICsgICAgICAgIGVycm9yX3NldGcoZXJycCwgImZhaWxlZCB0byBw YXJzZSBuYW1lICclcyciLCBuYW1lKTsKPiA+ID4gKyAgICAgICAgcmV0dXJuOwo+ID4gPiArICAg IH0KPiA+ID4gKwo+ID4gPiArICAgIHZkZXYgPSBxZGljdF9nZXRfdHJ5X3N0cihvcHRzLCAiZGV2 Iik7Cj4gPiA+ICsgICAgaWYgKCF2ZGV2KSB7Cj4gPiA+ICsgICAgICAgIGVycm9yX3NldGcoZXJy cCwgIm5vIGRldiBwYXJhbWV0ZXIiKTsKPiA+ID4gKyAgICAgICAgcmV0dXJuOwo+ID4gPiArICAg IH0KPiA+ID4gKwo+ID4gPiArICAgIGRldmljZV90eXBlID0gcWRpY3RfZ2V0X3RyeV9zdHIob3B0 cywgImRldmljZS10eXBlIik7Cj4gPiA+ICsgICAgaWYgKCFkZXZpY2VfdHlwZSkgewo+ID4gPiAr ICAgICAgICBlcnJvcl9zZXRnKGVycnAsICJubyBkZXZpY2UtdHlwZSBwYXJhbWV0ZXIiKTsKPiA+ ID4gKyAgICAgICAgcmV0dXJuOwo+ID4gPiArICAgIH0KPiA+ID4gKwo+ID4gPiArICAgIGlmICgh c3RyY21wKGRldmljZV90eXBlLCAiZGlzayIpKSB7Cj4gPiA+ICsgICAgICAgIHR5cGUgPSBUWVBF X1hFTl9ESVNLX0RFVklDRTsKPiA+ID4gKyAgICB9IGVsc2UgaWYgKCFzdHJjbXAoZGV2aWNlX3R5 cGUsICJjZHJvbSIpKSB7Cj4gPiA+ICsgICAgICAgIHR5cGUgPSBUWVBFX1hFTl9DRFJPTV9ERVZJ Q0U7Cj4gPiA+ICsgICAgfSBlbHNlIHsKPiA+ID4gKyAgICAgICAgZXJyb3Jfc2V0ZyhlcnJwLCAi aW52YWxpZCBkZXZpY2UtdHlwZSBwYXJhbWV0ZXIgJyVzJyIsCj4gPiBkZXZpY2VfdHlwZSk7Cj4g PiA+ICsgICAgICAgIHJldHVybjsKPiA+ID4gKyAgICB9Cj4gPiA+ICsKPiA+ID4gKyAgICB4ZW5f YmxvY2tfZHJpdmVfY3JlYXRlKHZkZXYsIGRldmljZV90eXBlLCBvcHRzLCAmbG9jYWxfZXJyKTsK PiA+ID4gKyAgICBpZiAobG9jYWxfZXJyKSB7Cj4gPiA+ICsgICAgICAgIGVycm9yX3Byb3BhZ2F0 ZShlcnJwLCBsb2NhbF9lcnIpOwo+ID4gPiArICAgICAgICByZXR1cm47Cj4gPiA+ICsgICAgfQo+ ID4gPiArCj4gPiA+ICsgICAgYmxrID0gYmxrX2J5X25hbWUodmRldik7Cj4gPiA+ICsgICAgZ19h c3NlcnQoYmxrKTsKPiA+ID4gKwo+ID4gPiArICAgIGlvdGhyZWFkID0gaW90aHJlYWRfY3JlYXRl KHZkZXYsICZsb2NhbF9lcnIpOwo+ID4gPiArICAgIGlmIChsb2NhbF9lcnIpIHsKPiA+ID4gKyAg ICAgICAgZXJyb3JfcHJvcGFnYXRlKGVycnAsIGxvY2FsX2Vycik7Cj4gPiA+ICsgICAgICAgIGdv dG8gdW5yZWY7Cj4gPiA+ICsgICAgfQo+ID4gPiArCj4gPiA+ICsgICAgZGV2ID0gcWRldl9jcmVh dGUoYnVzLCB0eXBlKTsKPiA+ID4gKyAgICBibG9ja2RldiA9IFhFTl9CTE9DS19ERVZJQ0UoZGV2 KTsKPiA+ID4gKwo+ID4gPiArICAgIHFkZXZfcHJvcF9zZXRfc3RyaW5nKGRldiwgInZkZXYiLCB2 ZGV2KTsKPiA+ID4gKyAgICBpZiAoYmxvY2tkZXYtPnZkZXYubnVtYmVyICE9IG51bWJlcikgewo+ ID4gPiArICAgICAgICBlcnJvcl9zZXRnKGVycnAsICJpbnZhbGlkIGRldiBwYXJhbWV0ZXIgJyVz JyIsIHZkZXYpOwo+ID4gPiArICAgICAgICBnb3RvIHVucmVmOwo+ID4gPiArICAgIH0KPiA+ID4g Kwo+ID4gPiArICAgIHFkZXZfcHJvcF9zZXRfZHJpdmUoZGV2LCAiZHJpdmUiLCBibGssICZsb2Nh bF9lcnIpOwo+ID4gPiArICAgIGlmIChsb2NhbF9lcnIpIHsKPiA+ID4gKyAgICAgICAgZXJyb3Jf cHJvcGFnYXRlX3ByZXBlbmQoZXJycCwgbG9jYWxfZXJyLCAiZmFpbGVkIHRvIHNldAo+ID4gJ2Ry aXZlJzogIik7Cj4gPiA+ICsgICAgICAgIGdvdG8gdW5yZWY7Cj4gPiA+ICsgICAgfQo+ID4gCj4g PiBTbyBoZXJlIHlvdSB3b3VsZCBuZWVkIHRvIHVzZSBzb21ldGhpbmcgbGlrZSB0aGlzOgo+ID4g Cj4gPiBvYmplY3RfcHJvcGVydHlfc2V0X3N0cihPQkpFQ1QoZGV2KSwgdmRldiwgImRyaXZlciIs ICZsb2NhbF9lcnIpOwo+ID4gCj4gPiA+ICsKPiA+ID4gKyAgICBibG9ja2Rldi0+YXV0b19pb3Ro cmVhZCA9IGlvdGhyZWFkOwo+ID4gPiArCj4gPiA+ICsgICAgb2JqZWN0X3Byb3BlcnR5X3NldF9i b29sKE9CSkVDVChkZXYpLCB0cnVlLCAicmVhbGl6ZWQiLAo+ID4gJmxvY2FsX2Vycik7Cj4gPiA+ ICsgICAgaWYgKGxvY2FsX2Vycikgewo+ID4gPiArICAgICAgICBlcnJvcl9wcm9wYWdhdGVfcHJl cGVuZChlcnJwLCBsb2NhbF9lcnIsCj4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICJpbml0aWFsaXphdGlvbiBvZiBkZXZpY2UgJXMgZmFpbGVkOiAiLAo+ID4gPiArICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICB0eXBlKTsKPiA+ID4gKyAgICAgICAgZ290byB1bnJl ZjsKPiA+ID4gKyAgICB9Cj4gPiA+ICsKPiA+ID4gKyAgICBibG9ja2Rldl9tYXJrX2F1dG9fZGVs KGJsayk7Cj4gPiAKPiA+IFlvdSBkb24ndCBuZWVkIHRoaXMgb25lIGFueSBtb3JlIHRoZW4gKGlm IHlvdSBsb29rIGludG8gdGhlIGRldGFpbHMsCj4gPiBpdCdzIG9uZSBvZiB0aGUgbW9yZSBjb25m dXNpbmcgcGFydHMgb2YgdGhlIGRyaXZlXyooKSBtYWdpYywgc28gaXQncwo+ID4gZ29vZCB0byBn ZXQgcmlkIG9mIGl0KS4gV2hlbiB5b3UgdXNlIHRoZSBhbm9ueW1vdXMgQmxvY2tCYWNrZW5kIGNy ZWF0ZWQKPiA+IGJ5IHRoZSBxZGV2IGRyaXZlIHByb3BlcnR5IChiZWNhdXNlIHlvdSBwYXNzZWQg aXQgYSBub2RlLW5hbWUgcmF0aGVyCj4gPiB0aGFuIGEgQmxvY2tCYWNrZW5kIG5hbWUpIG1lYW5z IHRoYXQgdGhlIEJsb2NrQmFja2VuZCBkaXNhcHBlYXJzCj4gPiB0b2dldGhlciB3aXRoIHRoZSBk cml2ZS4KPiA+IAo+IAo+IE9rLgo+IAo+ID4gTm90ZSB0aGF0IGV4cGxpY2l0bHkgY3JlYXRlZCBi bG9jayBub2RlcyBtdXN0IGFsc28gYmUgdW5yZWZlcmVuY2VkCj4gPiBleHBsaWNpdGx5IChiZHJ2 X29wZW4oKSBzaG91bGQgYmUgcGFpcmVkIHdpdGggYmRydl91bnJlZigpIGFuZAo+ID4gcW1wX2Js b2NrZGV2X2FkZCgpIHdpdGggcW1wX2Jsb2NrZGV2X2RlbCgpKS4gTWF5YmUgWGVuQmFja2VuZElu Zm8gbmVlZHMKPiA+IGEgLmRlc3Ryb3kgY2FsbGJhY2sgc28gd2UgY2FuIGRvIGRlc3RydWN0aW9u IHN5bW1ldHJpY2FsbHkgdG8gZGV2aWNlCj4gPiBjcmVhdGlvbj8KPiA+IAo+IAo+IFllcywgSSdk IHByb2JhYmx5IGp1c3QgYWRkIGEgY2FsbGJhY2sgZnVuY3Rpb24gcG9pbnRlciBpbnRvIFhlbkRl dmljZQo+IHdoaWNoIG9ubHkgZ2V0cyBzZXQgZm9yIGRldmljZXMgaW5zdGFudGlhdGVkIHZpYSB0 aGlzIG1lY2hhbmlzbS4KCkkgdGhpbmsgaXQncyBhIGJpdCBuaWNlciB0byBoYXZlIGl0IGluIFhl bkJhY2tlbmRJbmZvIGZvciBzeW1tZXRyeSBhbmQKYmVjYXVzZSB0aGF0IHNpdHMgb3V0c2lkZSB0 aGUgZGV2aWNlLCBzbyB0aGUgZGV2aWNlIGRvZXNuJ3QgZGVzdHJveSBpdHMKb3duIGJhY2tlbmQs IGp1c3QgbGlrZSBpdCBkb2Vzbid0IGNyZWF0ZSBpdCAoZXZlbiBpZiBpdCdzIHRoZSBzYW1lCnNv dXJjZSBmaWxlKS4KCkJ1dCB5ZXMsIHdoYXRldmVyIHdvcmtzIGJlc3QgZm9yIHlvdS4KCktldmlu CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpYZW4tZGV2 ZWwgbWFpbGluZyBsaXN0Clhlbi1kZXZlbEBsaXN0cy54ZW5wcm9qZWN0Lm9yZwpodHRwczovL2xp c3RzLnhlbnByb2plY3Qub3JnL21haWxtYW4vbGlzdGluZm8veGVuLWRldmVs From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41144) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gXSbJ-00086v-3Y for qemu-devel@nongnu.org; Thu, 13 Dec 2018 10:08:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gXSbH-0000gN-76 for qemu-devel@nongnu.org; Thu, 13 Dec 2018 10:08:33 -0500 Date: Thu, 13 Dec 2018 16:08:18 +0100 From: Kevin Wolf Message-ID: <20181213150818.GI5427@linux.fritz.box> References: <1544543862-9997-1-git-send-email-paul.durrant@citrix.com> <1544543862-9997-17-git-send-email-paul.durrant@citrix.com> <20181213115152.GA5427@linux.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v4 16/18] xen: automatically create XenBlockDevice-s List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Durrant Cc: "qemu-devel@nongnu.org" , "qemu-block@nongnu.org" , "xen-devel@lists.xenproject.org" , Max Reitz , Stefano Stabellini Am 13.12.2018 um 13:44 hat Paul Durrant geschrieben: > > Essentially, what I'm wondering is whether we have anything that could > > be treated more or less like another monitor besides QMP and HMP, which > > would internally work similar to HMP, i.e. map (almost) everything to > > QMP commands. > > Yes, it would be possible to have a separate 'compatibility' daemon to > watch xenstore and then formulate the correct sequence of QMP commands > to instantiate the backend, but that is more complicated and the right > answer of course is to have the toolstack send the QMP commands in the > first place. Okay, if someone is working on actually using QMP instead of xenstore, that's even better, of course. Disregard this point then. > > > + drive_new(drive_opts, IF_NONE, &local_err); > > > + if (local_err) { > > > + error_propagate_prepend(errp, local_err, > > > + "failed to create drive: "); > > > + goto done; > > > + } > > > > The other major point is that you're using the legacy drive_*() > > infrastructure, which should not only go away as soon as we can, but > > which is also full of magic and nasty surprises. > > > > I think the best way would be to create only a block node > > (BlockDriverState) here, and get an automatically created anonymous > > BlockBackend from the qdev drive property. > > > > There are two ways to achieve this: qmp_blockdev_add() would be optimal > > because that's a stable external interface. It would require you to > > specify a node-name (you already have the id parameter), and you'd use > > this node-name for the qdev drive property. > > > > qmp_blockdev_add() requires a BlockdevOptions object, which you can > > either construct manually in C or use a visitor to convert from an > > options QDict. Maybe in this case, converting from a QDict is better > > because otherwise you need special code for each block driver. > > > > I was using the legacy interfaces because this code is, as I said > above, supposed to be a mechanism only required for compatibility with > the way toolstacks currently operate (and so is essentially 'legacy') > but using the top-level QMP entry point to construct the does sound > do-able as long as the underlying file locking can still be avoided > with that mechanism. Since BlockdevOptions seems to be an > auto-generated structure, figuring out how to fill it in manually is > somewhat tricky so the QDict approach is preferable but I'll have to > figure out how to use a visitor to do the translation. You can grep for qobject_input_visitor_new() for examples. Some block drivers use this internally to convert .bdrv_create options to a QAPI object, it looks like this: /* Now get the QAPI type BlockdevCreateOptions */ v = qobject_input_visitor_new_flat_confused(qdict, errp); if (!v) { ret = -EINVAL; goto finish; } visit_type_BlockdevCreateOptions(v, NULL, &create_options, &local_err); visit_free(v); if (local_err) { error_propagate(errp, local_err); ret = -EINVAL; goto finish; } Obviously, you'd use visit_type_BlockdevOptions instead. You also might not need the _flat_confused variant, which is about QDicts where we don't know whether values are stored as their actual type or as strings. In your code, you have control over the types in the QDict, so this shouldn't be a problem. > > The other way would be calling bdrv_open() directly, which gives you a > > BlockDriverState, but it risks using legacy functionality that will be > > deprecated soon. Again, you'd take the node-name and pass it to the qdev > > drive option below. > > Yes, xen_disk does things this way but then we end up with legacy > block device and still fall foul of the assertions buried in the code. Yes and no. xen_disk is better in that it avoids the drive_* things (which internally call bdrv_open() anyway), but it's worse in that it directly assigns blkdev->blk instead of using the qdev property. What I meant here is that you create the BDS with bdrv_open(), but then you wouldn't assign it directly to some field in the device state, but just put the node-name of the BDS into the qdev property 'drive'. This would be almost like qmp_blockdev_add(), except without validation against the QAPI schema. But if using qmp_blockdev_add() is easy enough, that's preferable. > > > > > + > > > +done: > > > + g_free(drive_optstr); > > > + g_free(format); > > > + g_free(file); > > > +} > > > + > > > +static void xen_block_device_create(BusState *bus, const char *name, > > > + QDict *opts, Error **errp) > > > +{ > > > + unsigned long number; > > > + const char *vdev, *device_type; > > > + BlockBackend *blk = NULL; > > > + IOThread *iothread = NULL; > > > + DeviceState *dev = NULL; > > > + Error *local_err = NULL; > > > + const char *type; > > > + XenBlockDevice *blockdev; > > > + > > > + trace_xen_block_device_create(name); > > > + > > > + if (qemu_strtoul(name, NULL, 10, &number)) { > > > + error_setg(errp, "failed to parse name '%s'", name); > > > + return; > > > + } > > > + > > > + vdev = qdict_get_try_str(opts, "dev"); > > > + if (!vdev) { > > > + error_setg(errp, "no dev parameter"); > > > + return; > > > + } > > > + > > > + device_type = qdict_get_try_str(opts, "device-type"); > > > + if (!device_type) { > > > + error_setg(errp, "no device-type parameter"); > > > + return; > > > + } > > > + > > > + if (!strcmp(device_type, "disk")) { > > > + type = TYPE_XEN_DISK_DEVICE; > > > + } else if (!strcmp(device_type, "cdrom")) { > > > + type = TYPE_XEN_CDROM_DEVICE; > > > + } else { > > > + error_setg(errp, "invalid device-type parameter '%s'", > > device_type); > > > + return; > > > + } > > > + > > > + xen_block_drive_create(vdev, device_type, opts, &local_err); > > > + if (local_err) { > > > + error_propagate(errp, local_err); > > > + return; > > > + } > > > + > > > + blk = blk_by_name(vdev); > > > + g_assert(blk); > > > + > > > + iothread = iothread_create(vdev, &local_err); > > > + if (local_err) { > > > + error_propagate(errp, local_err); > > > + goto unref; > > > + } > > > + > > > + dev = qdev_create(bus, type); > > > + blockdev = XEN_BLOCK_DEVICE(dev); > > > + > > > + qdev_prop_set_string(dev, "vdev", vdev); > > > + if (blockdev->vdev.number != number) { > > > + error_setg(errp, "invalid dev parameter '%s'", vdev); > > > + goto unref; > > > + } > > > + > > > + qdev_prop_set_drive(dev, "drive", blk, &local_err); > > > + if (local_err) { > > > + error_propagate_prepend(errp, local_err, "failed to set > > 'drive': "); > > > + goto unref; > > > + } > > > > So here you would need to use something like this: > > > > object_property_set_str(OBJECT(dev), vdev, "driver", &local_err); > > > > > + > > > + blockdev->auto_iothread = iothread; > > > + > > > + object_property_set_bool(OBJECT(dev), true, "realized", > > &local_err); > > > + if (local_err) { > > > + error_propagate_prepend(errp, local_err, > > > + "initialization of device %s failed: ", > > > + type); > > > + goto unref; > > > + } > > > + > > > + blockdev_mark_auto_del(blk); > > > > You don't need this one any more then (if you look into the details, > > it's one of the more confusing parts of the drive_*() magic, so it's > > good to get rid of it). When you use the anonymous BlockBackend created > > by the qdev drive property (because you passed it a node-name rather > > than a BlockBackend name) means that the BlockBackend disappears > > together with the drive. > > > > Ok. > > > Note that explicitly created block nodes must also be unreferenced > > explicitly (bdrv_open() should be paired with bdrv_unref() and > > qmp_blockdev_add() with qmp_blockdev_del()). Maybe XenBackendInfo needs > > a .destroy callback so we can do destruction symmetrically to device > > creation? > > > > Yes, I'd probably just add a callback function pointer into XenDevice > which only gets set for devices instantiated via this mechanism. I think it's a bit nicer to have it in XenBackendInfo for symmetry and because that sits outside the device, so the device doesn't destroy its own backend, just like it doesn't create it (even if it's the same source file). But yes, whatever works best for you. Kevin