Patchwork [RFC,2/6] block: add bdrv_set_copy_on_read()

login
register
mail settings
Submitter Stefan Hajnoczi
Date Oct. 17, 2011, 3:47 p.m.
Message ID <1318866452-30026-3-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/120261/
State New
Headers show

Comments

Stefan Hajnoczi - Oct. 17, 2011, 3:47 p.m.
The bdrv_set_copy_on_read() function can be used to programmatically
enable or disable copy-on-read for a block device.  Later patches add
the actual copy-on-read logic.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c     |   17 +++++++++++++++++
 block.h     |    3 +++
 block_int.h |    1 +
 3 files changed, 21 insertions(+), 0 deletions(-)
Kevin Wolf - Nov. 2, 2011, 4:36 p.m.
Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
> The bdrv_set_copy_on_read() function can be used to programmatically
> enable or disable copy-on-read for a block device.  Later patches add
> the actual copy-on-read logic.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block.c     |   17 +++++++++++++++++
>  block.h     |    3 +++
>  block_int.h |    1 +
>  3 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 2d2c62a..e624ac3 100644
> --- a/block.c
> +++ b/block.c
> @@ -464,6 +464,18 @@ int bdrv_parse_cache_flags(const char *mode, int *flags)
>      return 0;
>  }
>  
> +void bdrv_set_copy_on_read(BlockDriverState *bs, int enable)

bool enable

> +{
> +    if (bs->copy_on_read != enable) {
> +        if (enable) {
> +            bdrv_start_request_tracking(bs);
> +        } else {
> +            bdrv_stop_request_tracking(bs);
> +        }
> +    }
> +    bs->copy_on_read = enable;
> +}
> +
>  /*
>   * Common part for opening disk images and files
>   */
> @@ -483,6 +495,11 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
>      bs->open_flags = flags;
>      bs->buffer_alignment = 512;
>  
> +    bs->copy_on_read = 0;

I think it should rather be reset on close. We can assert() it here.

> +    if (flags & BDRV_O_RDWR) {
> +        bdrv_set_copy_on_read(bs, !!(flags & BDRV_O_COPY_ON_READ));
> +    }
> +
>      pstrcpy(bs->filename, sizeof(bs->filename), filename);
>  
>      if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
> diff --git a/block.h b/block.h
> index e77988e..cb9832c 100644
> --- a/block.h
> +++ b/block.h
> @@ -61,6 +61,7 @@ typedef struct BlockDevOps {
>  #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_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
>  
>  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
>  
> @@ -291,6 +292,8 @@ void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
>                        int nr_sectors);
>  int64_t bdrv_get_dirty_count(BlockDriverState *bs);
>  
> +void bdrv_set_copy_on_read(BlockDriverState *bs, int enable);
> +
>  void bdrv_set_in_use(BlockDriverState *bs, int in_use);
>  int bdrv_in_use(BlockDriverState *bs);
>  
> diff --git a/block_int.h b/block_int.h
> index 87ce8b4..8eb4795 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -160,6 +160,7 @@ struct BlockDriverState {
>      int encrypted; /* if true, the media is encrypted */
>      int valid_key; /* if true, a valid encryption key has been set */
>      int sg;        /* if true, the device is a /dev/sg* */
> +    int copy_on_read; /* if true, copy read backing sectors into image */

bool.

Kevin
Stefan Hajnoczi - Nov. 3, 2011, 8:01 a.m.
On Wed, Nov 2, 2011 at 4:36 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
>> The bdrv_set_copy_on_read() function can be used to programmatically
>> enable or disable copy-on-read for a block device.  Later patches add
>> the actual copy-on-read logic.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>> ---
>>  block.c     |   17 +++++++++++++++++
>>  block.h     |    3 +++
>>  block_int.h |    1 +
>>  3 files changed, 21 insertions(+), 0 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 2d2c62a..e624ac3 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -464,6 +464,18 @@ int bdrv_parse_cache_flags(const char *mode, int *flags)
>>      return 0;
>>  }
>>
>> +void bdrv_set_copy_on_read(BlockDriverState *bs, int enable)
>
> bool enable

Thanks for pointing this out.

>> +{
>> +    if (bs->copy_on_read != enable) {
>> +        if (enable) {
>> +            bdrv_start_request_tracking(bs);
>> +        } else {
>> +            bdrv_stop_request_tracking(bs);
>> +        }
>> +    }
>> +    bs->copy_on_read = enable;
>> +}
>> +
>>  /*
>>   * Common part for opening disk images and files
>>   */
>> @@ -483,6 +495,11 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
>>      bs->open_flags = flags;
>>      bs->buffer_alignment = 512;
>>
>> +    bs->copy_on_read = 0;
>
> I think it should rather be reset on close. We can assert() it here.

Okay.

Stefan

Patch

diff --git a/block.c b/block.c
index 2d2c62a..e624ac3 100644
--- a/block.c
+++ b/block.c
@@ -464,6 +464,18 @@  int bdrv_parse_cache_flags(const char *mode, int *flags)
     return 0;
 }
 
+void bdrv_set_copy_on_read(BlockDriverState *bs, int enable)
+{
+    if (bs->copy_on_read != enable) {
+        if (enable) {
+            bdrv_start_request_tracking(bs);
+        } else {
+            bdrv_stop_request_tracking(bs);
+        }
+    }
+    bs->copy_on_read = enable;
+}
+
 /*
  * Common part for opening disk images and files
  */
@@ -483,6 +495,11 @@  static int bdrv_open_common(BlockDriverState *bs, const char *filename,
     bs->open_flags = flags;
     bs->buffer_alignment = 512;
 
+    bs->copy_on_read = 0;
+    if (flags & BDRV_O_RDWR) {
+        bdrv_set_copy_on_read(bs, !!(flags & BDRV_O_COPY_ON_READ));
+    }
+
     pstrcpy(bs->filename, sizeof(bs->filename), filename);
 
     if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
diff --git a/block.h b/block.h
index e77988e..cb9832c 100644
--- a/block.h
+++ b/block.h
@@ -61,6 +61,7 @@  typedef struct BlockDevOps {
 #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_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 
@@ -291,6 +292,8 @@  void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
                       int nr_sectors);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs);
 
+void bdrv_set_copy_on_read(BlockDriverState *bs, int enable);
+
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
diff --git a/block_int.h b/block_int.h
index 87ce8b4..8eb4795 100644
--- a/block_int.h
+++ b/block_int.h
@@ -160,6 +160,7 @@  struct BlockDriverState {
     int encrypted; /* if true, the media is encrypted */
     int valid_key; /* if true, a valid encryption key has been set */
     int sg;        /* if true, the device is a /dev/sg* */
+    int copy_on_read; /* if true, copy read backing sectors into image */
 
     BlockDriver *drv; /* NULL means no media */
     void *opaque;