From mboxrd@z Thu Jan  1 00:00:00 1970
From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Subject: Re: [PATCH v2 08/18] nvdimm: init backend memory mapping and config
 data area
Date: Thu, 17 Sep 2015 16:21:34 +0800
Message-ID: <55FA780E.5020305@linux.intel.com>
References: <1439563931-12352-1-git-send-email-guangrong.xiao@linux.intel.com>
 <1439563931-12352-9-git-send-email-guangrong.xiao@linux.intel.com>
 <20150825160353.GD8344@stefanha-thinkpad.redhat.com>
 <55F841EC.1060809@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: imammedo@redhat.com, gleb@kernel.org, mtosatti@redhat.com,
	stefanha@redhat.com, mst@redhat.com, rth@twiddle.net,
	ehabkost@redhat.com, kvm@vger.kernel.org, qemu-devel@nongnu.org
To: Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mga09.intel.com ([134.134.136.24]:50921 "EHLO mga09.intel.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753881AbbIQI1e (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 17 Sep 2015 04:27:34 -0400
In-Reply-To: <55F841EC.1060809@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



On 09/16/2015 12:06 AM, Paolo Bonzini wrote:
>
>
> On 25/08/2015 18:03, Stefan Hajnoczi wrote:
>>>>
>>>> +static uint64_t get_file_size(int fd)
>>>> +{
>>>> +    struct stat stat_buf;
>>>> +    uint64_t size;
>>>> +
>>>> +    if (fstat(fd, &stat_buf) < 0) {
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    if (S_ISREG(stat_buf.st_mode)) {
>>>> +        return stat_buf.st_size;
>>>> +    }
>>>> +
>>>> +    if (S_ISBLK(stat_buf.st_mode) && !ioctl(fd, BLKGETSIZE64, &size)) {
>>>> +        return size;
>>>> +    }
>> #ifdef __linux__ for ioctl(fd, BLKGETSIZE64, &size)?
>>
>> There is nothing Linux-specific about emulating NVDIMMs so this code
>> should compile on all platforms.
>
> The code from block/raw-posix.c and block/raw-win32.c's raw_getlength
> should probably be extracted to a new function in utils/, and reused here.
>

The function you pointed out is really complex - it mixed 9 platforms and each
platform has its own specific implementation. It is hard for us to verify the
change.

I'd prefer to make it for Linux specific first then share it to other platforms
if it's needed in the future.