From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46573) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZX4og-0000Qq-17 for qemu-devel@nongnu.org; Wed, 02 Sep 2015 05:58:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZX4ob-00033H-Iy for qemu-devel@nongnu.org; Wed, 02 Sep 2015 05:58:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60182) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZX4ob-00033D-E6 for qemu-devel@nongnu.org; Wed, 02 Sep 2015 05:58:49 -0400 Date: Wed, 2 Sep 2015 11:58:45 +0200 From: Igor Mammedov Message-ID: <20150902115845.01472189@nial.brq.redhat.com> In-Reply-To: <1439563931-12352-7-git-send-email-guangrong.xiao@linux.intel.com> References: <1439563931-12352-1-git-send-email-guangrong.xiao@linux.intel.com> <1439563931-12352-7-git-send-email-guangrong.xiao@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 06/18] pc: implement NVDIMM device abstract List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiao Guangrong Cc: ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, rth@twiddle.net On Fri, 14 Aug 2015 22:51:59 +0800 Xiao Guangrong wrote: > Introduce "pc-nvdimm" device and it has two parameters: Why do you use prefix "pc-", I suppose we potentially could use this device not only with x86 targets but with other targets as well. I'd just drop 'pc' prefix through out patchset. > - @file, which is the backed memory file for NVDIMM device Could you try to split device into backend/frontend parts, like it's done with pc-dimm. As I understand it's preferred way to implement this kind of devices. Then you could reuse memory backends that we already have including file backend. So CLI could look like: -object memory-backend-file,id=mem0,file=/storage/foo -device nvdimm,memdev=mem0,configdata=on > > - @configdata, specify if we need to reserve 128k at the end of > @file for nvdimm device's config data. Default is false > > If @configdata is false, Qemu will build a static and readonly > namespace in memory and use it serveing for > DSM GET_CONFIG_SIZE/GET_CONFIG_DATA requests. > This is good for the user who want to pass whole nvdimm device > and make its data is complete visible to guest > > We can use "-device pc-nvdimm,file=/dev/pmem,configdata" in the > Qemu command to create NVDIMM device for the guest PS: please try to fix commit message spelling/grammar wise. [...] > +++ b/hw/mem/nvdimm/pc-nvdimm.c > @@ -0,0 +1,99 @@ > +/* > + * NVDIMM (A Non-Volatile Dual In-line Memory Module) Virtualization Implement s/Implement/Implementation/ in all new files an maybe s/NVDIMM (A // as it's reduntant [...] > +static bool has_configdata(Object *obj, Error **errp) > +{ > + PCNVDIMMDevice *nvdimm = PC_NVDIMM(obj); > + > + return nvdimm->configdata; > +} > + > +static void set_configdata(Object *obj, bool value, Error **errp) > +{ > + PCNVDIMMDevice *nvdimm = PC_NVDIMM(obj); > + > + nvdimm->configdata = value; > +} usually for property setters/getters we use form: "device_prefix"_[g|s]et_foo so nvdim_get_configdata ... [...]