Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-09 Thread Eduardo Habkost 
On Mon, Nov 09, 2015 at 01:58:27PM +0800, Xiao Guangrong wrote:
> 
> 
> On 11/06/2015 11:54 PM, Eduardo Habkost wrote:
> >On Mon, Nov 02, 2015 at 05:13:14PM +0800, Xiao Guangrong wrote:
> >>lseek can not work for all block devices as the man page says:
> >>| Some devices are incapable of seeking and POSIX does not specify
> >>| which devices must support lseek().
> >>
> >>This patch tries to add the support on Linux by using BLKGETSIZE64
> >>ioctl
> >>
> >>Signed-off-by: Xiao Guangrong 
> >
> >On which cases is this patch necessary? Do you know any examples of
> >Linux block devices that won't work with lseek(SEEK_END)?
> 
> To be honest, i have not checked all block device, this patch was made
> based on the man page. However, i do not mind drop this patch (and maybe
> other patches) to make this pachset smaller. BLKGETSIZE64 can be added
> in the future if we meet such device.

By looking at the Linux source code implementing BLKGETSIZE64, it looks
like it should work for all block devices where SEEK_END works:

* BLKGETSIZE64 returns i_size_read(bdev->bd_inode)
  (block/ioctl.c:blkdev_ioctl())
* llseek(SEEK_END) uses i_size_read(bd_inode) as the offset
  (fs/block_dev.c:block_llseek())

That's probably why raw_getlength() never needed a Linux-specific
BLKGETSIZE call.

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-08 Thread Xiao Guangrong 



On 11/06/2015 11:54 PM, Eduardo Habkost wrote:

On Mon, Nov 02, 2015 at 05:13:14PM +0800, Xiao Guangrong wrote:

lseek can not work for all block devices as the man page says:
| Some devices are incapable of seeking and POSIX does not specify
| which devices must support lseek().

This patch tries to add the support on Linux by using BLKGETSIZE64
ioctl

Signed-off-by: Xiao Guangrong 


On which cases is this patch necessary? Do you know any examples of
Linux block devices that won't work with lseek(SEEK_END)?


To be honest, i have not checked all block device, this patch was made
based on the man page. However, i do not mind drop this patch (and maybe
other patches) to make this pachset smaller. BLKGETSIZE64 can be added
in the future if we meet such device.


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-06 Thread Eduardo Habkost 
On Tue, Nov 03, 2015 at 12:21:30AM +0800, Xiao Guangrong wrote:
> On 11/03/2015 12:11 AM, Vladimir Sementsov-Ogievskiy wrote:
> >On 02.11.2015 12:13, Xiao Guangrong wrote:
> >>lseek can not work for all block devices as the man page says:
> >>| Some devices are incapable of seeking and POSIX does not specify
> >>| which devices must support lseek().
> >>
> >>This patch tries to add the support on Linux by using BLKGETSIZE64
> >>ioctl
> >>
> >>Signed-off-by: Xiao Guangrong 
> >>---
> >>  util/osdep.c | 20 
> >>  1 file changed, 20 insertions(+)
> >>
> >>diff --git a/util/osdep.c b/util/osdep.c
> >>index 5a61e19..b20c793 100644
> >>--- a/util/osdep.c
> >>+++ b/util/osdep.c
> >>@@ -45,6 +45,11 @@
> >>  extern int madvise(caddr_t, size_t, int);
> >>  #endif
> >>+#ifdef CONFIG_LINUX
> >>+#include 
> >>+#include 
> >>+#endif
> >>+
> >>  #include "qemu-common.h"
> >>  #include "qemu/sockets.h"
> >>  #include "qemu/error-report.h"
> >>@@ -433,6 +438,21 @@ int64_t qemu_fd_getlength(int fd)
> >>  {
> >>  int64_t size;
> >>+#ifdef CONFIG_LINUX
> >>+struct stat stat_buf;
> >>+if (fstat(fd, _buf) < 0) {
> >>+return -errno;
> >>+}
> >>+
> >>+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, )) {
> >>+/* The size of block device is larger than max int64_t? */
> >>+if (size < 0) {
> >>+return -EOVERFLOW;
> >>+}
> >>+return size;
> >>+}
> >>+#endif
> >>+
> >>  size = lseek(fd, 0, SEEK_END);
> >>  if (size < 0) {
> >>  return -errno;
> >
> >Reviewed-by: Vladimir Sementsov-Ogievskiy 
> >
> >just a question: is there any use for stat.st_size ? Is it always worse then 
> >lseek?
> 
> The man page says:
> The  st_size field gives the size of the file (if it is a regular file or a 
> symbolic link)
> in bytes.  The size of a symbolic link is the length of the pathname it 
> contains, without a
> terminating null byte.
> 
> So it can not work on symbolic link.

stat() and fstat() will get you information for the file pointed by the
symlink. You will get the symlink information only if you use lstat().

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-06 Thread Eduardo Habkost 
As this patch affects raw_getlength(), CCing the raw block driver
maintainer and the qemu-block mailing list.

On Mon, Nov 02, 2015 at 05:13:14PM +0800, Xiao Guangrong wrote:
> lseek can not work for all block devices as the man page says:
> | Some devices are incapable of seeking and POSIX does not specify
> | which devices must support lseek().
> 
> This patch tries to add the support on Linux by using BLKGETSIZE64
> ioctl
> 
> Signed-off-by: Xiao Guangrong 
> ---
>  util/osdep.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/util/osdep.c b/util/osdep.c
> index 5a61e19..b20c793 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -45,6 +45,11 @@
>  extern int madvise(caddr_t, size_t, int);
>  #endif
>  
> +#ifdef CONFIG_LINUX
> +#include 
> +#include 
> +#endif
> +
>  #include "qemu-common.h"
>  #include "qemu/sockets.h"
>  #include "qemu/error-report.h"
> @@ -433,6 +438,21 @@ int64_t qemu_fd_getlength(int fd)
>  {
>  int64_t size;
>  
> +#ifdef CONFIG_LINUX
> +struct stat stat_buf;
> +if (fstat(fd, _buf) < 0) {
> +return -errno;
> +}
> +
> +if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, )) {
> +/* The size of block device is larger than max int64_t? */
> +if (size < 0) {
> +return -EOVERFLOW;
> +}
> +return size;
> +}
> +#endif
> +
>  size = lseek(fd, 0, SEEK_END);
>  if (size < 0) {
>  return -errno;
> -- 
> 1.8.3.1
> 

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-06 Thread Eduardo Habkost 
On Mon, Nov 02, 2015 at 05:13:14PM +0800, Xiao Guangrong wrote:
> lseek can not work for all block devices as the man page says:
> | Some devices are incapable of seeking and POSIX does not specify
> | which devices must support lseek().
> 
> This patch tries to add the support on Linux by using BLKGETSIZE64
> ioctl
> 
> Signed-off-by: Xiao Guangrong 

On which cases is this patch necessary? Do you know any examples of
Linux block devices that won't work with lseek(SEEK_END)?

> ---
>  util/osdep.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/util/osdep.c b/util/osdep.c
> index 5a61e19..b20c793 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -45,6 +45,11 @@
>  extern int madvise(caddr_t, size_t, int);
>  #endif
>  
> +#ifdef CONFIG_LINUX
> +#include 
> +#include 
> +#endif
> +
>  #include "qemu-common.h"
>  #include "qemu/sockets.h"
>  #include "qemu/error-report.h"
> @@ -433,6 +438,21 @@ int64_t qemu_fd_getlength(int fd)
>  {
>  int64_t size;
>  
> +#ifdef CONFIG_LINUX
> +struct stat stat_buf;
> +if (fstat(fd, _buf) < 0) {
> +return -errno;
> +}
> +
> +if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, )) {
> +/* The size of block device is larger than max int64_t? */
> +if (size < 0) {
> +return -EOVERFLOW;
> +}
> +return size;
> +}
> +#endif
> +
>  size = lseek(fd, 0, SEEK_END);
>  if (size < 0) {
>  return -errno;
> -- 
> 1.8.3.1
> 

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-02 Thread Vladimir Sementsov-Ogievskiy 

On 02.11.2015 12:13, Xiao Guangrong wrote:

lseek can not work for all block devices as the man page says:
| Some devices are incapable of seeking and POSIX does not specify
| which devices must support lseek().

This patch tries to add the support on Linux by using BLKGETSIZE64
ioctl

Signed-off-by: Xiao Guangrong 
---
  util/osdep.c | 20 
  1 file changed, 20 insertions(+)

diff --git a/util/osdep.c b/util/osdep.c
index 5a61e19..b20c793 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -45,6 +45,11 @@
  extern int madvise(caddr_t, size_t, int);
  #endif
  
+#ifdef CONFIG_LINUX

+#include 
+#include 
+#endif
+
  #include "qemu-common.h"
  #include "qemu/sockets.h"
  #include "qemu/error-report.h"
@@ -433,6 +438,21 @@ int64_t qemu_fd_getlength(int fd)
  {
  int64_t size;
  
+#ifdef CONFIG_LINUX

+struct stat stat_buf;
+if (fstat(fd, _buf) < 0) {
+return -errno;
+}
+
+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, )) {
+/* The size of block device is larger than max int64_t? */
+if (size < 0) {
+return -EOVERFLOW;
+}
+return size;
+}
+#endif
+
  size = lseek(fd, 0, SEEK_END);
  if (size < 0) {
  return -errno;


Reviewed-by: Vladimir Sementsov-Ogievskiy 

just a question: is there any use for stat.st_size ? Is it always worse 
then lseek? Does it work for blk?


also, "This patch tries to add..". Hmm. It looks like this patch is not 
sure about will it success. I'd prefer "This patch adds", but this is 
not important


--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-02 Thread Xiao Guangrong 



On 11/03/2015 12:11 AM, Vladimir Sementsov-Ogievskiy wrote:

On 02.11.2015 12:13, Xiao Guangrong wrote:

lseek can not work for all block devices as the man page says:
| Some devices are incapable of seeking and POSIX does not specify
| which devices must support lseek().

This patch tries to add the support on Linux by using BLKGETSIZE64
ioctl

Signed-off-by: Xiao Guangrong 
---
  util/osdep.c | 20 
  1 file changed, 20 insertions(+)

diff --git a/util/osdep.c b/util/osdep.c
index 5a61e19..b20c793 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -45,6 +45,11 @@
  extern int madvise(caddr_t, size_t, int);
  #endif
+#ifdef CONFIG_LINUX
+#include 
+#include 
+#endif
+
  #include "qemu-common.h"
  #include "qemu/sockets.h"
  #include "qemu/error-report.h"
@@ -433,6 +438,21 @@ int64_t qemu_fd_getlength(int fd)
  {
  int64_t size;
+#ifdef CONFIG_LINUX
+struct stat stat_buf;
+if (fstat(fd, _buf) < 0) {
+return -errno;
+}
+
+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, )) {
+/* The size of block device is larger than max int64_t? */
+if (size < 0) {
+return -EOVERFLOW;
+}
+return size;
+}
+#endif
+
  size = lseek(fd, 0, SEEK_END);
  if (size < 0) {
  return -errno;


Reviewed-by: Vladimir Sementsov-Ogievskiy 

just a question: is there any use for stat.st_size ? Is it always worse then 
lseek?


The man page says:
The  st_size field gives the size of the file (if it is a regular file or a 
symbolic link)
in bytes.  The size of a symbolic link is the length of the pathname it 
contains, without a
terminating null byte.

So it can not work on symbolic link.


Does it work for
blk?



Quickly checked with a program written by myself and 'stat' command, the answer 
is NO. :)


also, "This patch tries to add..". Hmm. It looks like this patch is not sure 
about will it success.
I'd prefer "This patch adds", but this is not important



Thanks for your sharing. I did not know the different, now, i got it. :)

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 12/35] util: let qemu_fd_getlength support block device

2015-11-02 Thread Xiao Guangrong 
lseek can not work for all block devices as the man page says:
| Some devices are incapable of seeking and POSIX does not specify
| which devices must support lseek().

This patch tries to add the support on Linux by using BLKGETSIZE64
ioctl

Signed-off-by: Xiao Guangrong 
---
 util/osdep.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/util/osdep.c b/util/osdep.c
index 5a61e19..b20c793 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -45,6 +45,11 @@
 extern int madvise(caddr_t, size_t, int);
 #endif
 
+#ifdef CONFIG_LINUX
+#include 
+#include 
+#endif
+
 #include "qemu-common.h"
 #include "qemu/sockets.h"
 #include "qemu/error-report.h"
@@ -433,6 +438,21 @@ int64_t qemu_fd_getlength(int fd)
 {
 int64_t size;
 
+#ifdef CONFIG_LINUX
+struct stat stat_buf;
+if (fstat(fd, _buf) < 0) {
+return -errno;
+}
+
+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, )) {
+/* The size of block device is larger than max int64_t? */
+if (size < 0) {
+return -EOVERFLOW;
+}
+return size;
+}
+#endif
+
 size = lseek(fd, 0, SEEK_END);
 if (size < 0) {
 return -errno;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html