Patchwork [5/5] drive_open: Add invalidate option for block devices

login
register
mail settings
Submitter Juan Quintela
Date Jan. 4, 2011, 2:33 p.m.
Message ID <95fe2a778152f50d942dcb17872927247195d0b1.1294150511.git.quintela@redhat.com>
Download mbox | patch
Permalink /patch/77480/
State New
Headers show

Comments

Juan Quintela - Jan. 4, 2011, 2:33 p.m.
Linux allows to invalidate block devices.  This is needed for the incoming
migration part.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block.h           |    2 ++
 block/raw-posix.c |   24 ++++++++++++++++++++++++
 blockdev.c        |    9 +++++----
 3 files changed, 31 insertions(+), 4 deletions(-)
Blue Swirl - Jan. 4, 2011, 7:06 p.m.
On Tue, Jan 4, 2011 at 2:33 PM, Juan Quintela <quintela@redhat.com> wrote:
> Linux allows to invalidate block devices.  This is needed for the incoming
> migration part.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  block.h           |    2 ++
>  block/raw-posix.c |   24 ++++++++++++++++++++++++
>  blockdev.c        |    9 +++++----
>  3 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/block.h b/block.h
> index f923add..5ac96a5 100644
> --- a/block.h
> +++ b/block.h
> @@ -34,6 +34,8 @@ typedef struct QEMUSnapshotInfo {
>  #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
>  #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
>  #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
> +#define BDRV_O_INVALIDATE  0x0400 /* invalidate buffer cache for this device.
> +                                     re-read things from server */
>
>  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 6b72470..9439cf1 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -51,6 +51,7 @@
>  #include <sys/param.h>
>  #include <linux/cdrom.h>
>  #include <linux/fd.h>
> +#include <linux/fs.h>
>  #endif
>  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
>  #include <signal.h>
> @@ -168,6 +169,29 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
>     s->fd = fd;
>     s->aligned_buf = NULL;
>
> +#ifdef __linux__
> +    if ((bdrv_flags & BDRV_O_INVALIDATE)) {
> +        struct stat buf;
> +        int res;
> +
> +        res = fstat(fd, &buf);
> +
> +        if (res < 0) {
> +            return -errno;
> +        }
> +
> +        if (S_ISBLK(buf.st_mode)) {
> +            printf("we are in a block device: %s\n", filename);

Leftover debugging?
Juan Quintela - Jan. 4, 2011, 7:23 p.m.
Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Jan 4, 2011 at 2:33 PM, Juan Quintela <quintela@redhat.com> wrote:
>> Linux allows to invalidate block devices.  This is needed for the incoming
>> migration part.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  block.h           |    2 ++
>>  block/raw-posix.c |   24 ++++++++++++++++++++++++
>>  blockdev.c        |    9 +++++----
>>  3 files changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/block.h b/block.h
>> index f923add..5ac96a5 100644
>> --- a/block.h
>> +++ b/block.h
>> @@ -34,6 +34,8 @@ typedef struct QEMUSnapshotInfo {
>>  #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
>>  #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
>>  #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
>> +#define BDRV_O_INVALIDATE  0x0400 /* invalidate buffer cache for this device.
>> +                                     re-read things from server */
>>
>>  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
>>
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index 6b72470..9439cf1 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -51,6 +51,7 @@
>>  #include <sys/param.h>
>>  #include <linux/cdrom.h>
>>  #include <linux/fd.h>
>> +#include <linux/fs.h>
>>  #endif
>>  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
>>  #include <signal.h>
>> @@ -168,6 +169,29 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
>>     s->fd = fd;
>>     s->aligned_buf = NULL;
>>
>> +#ifdef __linux__
>> +    if ((bdrv_flags & BDRV_O_INVALIDATE)) {
>> +        struct stat buf;
>> +        int res;
>> +
>> +        res = fstat(fd, &buf);
>> +
>> +        if (res < 0) {
>> +            return -errno;
>> +        }
>> +
>> +        if (S_ISBLK(buf.st_mode)) {
>> +            printf("we are in a block device: %s\n", filename);
>
> Leftover debugging?

ouch, yes.

thanks for the spotting.

Later, Juan.
Christoph Hellwig - Jan. 7, 2011, 8:38 a.m.
On Tue, Jan 04, 2011 at 03:33:30PM +0100, Juan Quintela wrote:
> Linux allows to invalidate block devices.  This is needed for the incoming
> migration part.

It's not.  The only thing that makes migration on block devices safe is
using O_DIRECT, aka cache=none.

Patch

diff --git a/block.h b/block.h
index f923add..5ac96a5 100644
--- a/block.h
+++ b/block.h
@@ -34,6 +34,8 @@  typedef struct QEMUSnapshotInfo {
 #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
 #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
 #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
+#define BDRV_O_INVALIDATE  0x0400 /* invalidate buffer cache for this device.
+                                     re-read things from server */

 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6b72470..9439cf1 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -51,6 +51,7 @@ 
 #include <sys/param.h>
 #include <linux/cdrom.h>
 #include <linux/fd.h>
+#include <linux/fs.h>
 #endif
 #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
 #include <signal.h>
@@ -168,6 +169,29 @@  static int raw_open_common(BlockDriverState *bs, const char *filename,
     s->fd = fd;
     s->aligned_buf = NULL;

+#ifdef __linux__
+    if ((bdrv_flags & BDRV_O_INVALIDATE)) {
+        struct stat buf;
+        int res;
+
+        res = fstat(fd, &buf);
+
+        if (res < 0) {
+            return -errno;
+        }
+
+        if (S_ISBLK(buf.st_mode)) {
+            printf("we are in a block device: %s\n", filename);
+            res = ioctl(fd, BLKFLSBUF, 0);
+            if (res < 0) {
+                fprintf(stderr, "qemu: buffer invalidation of %s"
+                        " failed with error %d\n", filename, errno);
+                return -errno;
+            }
+        }
+    }
+#endif /* __linux__ */
+
     if ((bdrv_flags & BDRV_O_NOCACHE)) {
         /*
          * Allocate a buffer for read/modify/write cycles.  Chose the size
diff --git a/blockdev.c b/blockdev.c
index 3f3df7a..94920b8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -137,9 +137,10 @@  static int parse_block_error_action(const char *buf, int is_read)
     }
 }

-static int drive_open(DriveInfo *dinfo)
+static int drive_open(DriveInfo *dinfo, int extra_flags)
 {
-    int res = bdrv_open(dinfo->bdrv, dinfo->file, dinfo->bdrv_flags, dinfo->drv);
+    int res = bdrv_open(dinfo->bdrv, dinfo->file,
+                        dinfo->bdrv_flags | extra_flags, dinfo->drv);

     if (res < 0) {
         fprintf(stderr, "qemu: could not open disk image %s: %s\n",
@@ -156,7 +157,7 @@  int drives_reinit(void)
         if (dinfo->opened && !bdrv_is_read_only(dinfo->bdrv)) {
             int res;
             bdrv_close(dinfo->bdrv);
-            res = drive_open(dinfo);
+            res = drive_open(dinfo, BDRV_O_INVALIDATE);
             if (res) {
 		    fprintf(stderr, "qemu: re-open of %s failed wth error %d\n",
 			    dinfo->file, res);
@@ -522,7 +523,7 @@  DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     dinfo->drv = drv;
     dinfo->opened = 1;

-    if (drive_open(dinfo) < 0) {
+    if (drive_open(dinfo, 0) < 0) {
         goto error;
     }