diff mbox

[PATCHv3,06/20] block: add BlockLimits structure to BlockDriverState

Message ID 1380029714-5239-7-git-send-email-pl@kamp.de
State New
Headers show

Commit Message

Peter Lieven Sept. 24, 2013, 1:35 p.m. UTC
this patch adds BlockLimits which introduces discard and write_zeroes
limits and alignment information to the BlockDriverState.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 include/block/block_int.h |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Eric Blake Oct. 2, 2013, 3:53 p.m. UTC | #1
On 09/24/2013 07:35 AM, Peter Lieven wrote:
> this patch adds BlockLimits which introduces discard and write_zeroes
> limits and alignment information to the BlockDriverState.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/block/block_int.h |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

>  
> +struct BlockLimits {

Should this be a typedef?  Either in include/qemu/typedefs.h (where
BlockDriverState is listed), or locally (see BdrvTrackedRequest in the
same file for an example)?

> @@ -280,6 +294,9 @@ struct BlockDriverState {
>      uint64_t total_time_ns[BDRV_MAX_IOTYPE];
>      uint64_t wr_highest_sector;
>  
> +    /* I/O Limits */
> +    struct BlockLimits bl;
> +

All other struct/pointer-to-struct members in BlockDriverState are
listed by typedef name, rather than calling out 'struct foo'.

My question is one of style/consistency, not of C correctness; so unless
a maintainer actually agrees that a typedef change is needed so that you
comply with project coding standards, feel free to add:

Reviewed-by: Eric Blake <eblake@redhat.com>
Stefan Hajnoczi Oct. 7, 2013, 8:10 a.m. UTC | #2
On Wed, Oct 02, 2013 at 09:53:01AM -0600, Eric Blake wrote:
> On 09/24/2013 07:35 AM, Peter Lieven wrote:
> > this patch adds BlockLimits which introduces discard and write_zeroes
> > limits and alignment information to the BlockDriverState.
> > 
> > Signed-off-by: Peter Lieven <pl@kamp.de>
> > ---
> >  include/block/block_int.h |   17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> 
> >  
> > +struct BlockLimits {
> 
> Should this be a typedef?  Either in include/qemu/typedefs.h (where
> BlockDriverState is listed), or locally (see BdrvTrackedRequest in the
> same file for an example)?
> 
> > @@ -280,6 +294,9 @@ struct BlockDriverState {
> >      uint64_t total_time_ns[BDRV_MAX_IOTYPE];
> >      uint64_t wr_highest_sector;
> >  
> > +    /* I/O Limits */
> > +    struct BlockLimits bl;
> > +
> 
> All other struct/pointer-to-struct members in BlockDriverState are
> listed by typedef name, rather than calling out 'struct foo'.
> 
> My question is one of style/consistency, not of C correctness; so unless
> a maintainer actually agrees that a typedef change is needed so that you
> comply with project coding standards, feel free to add:

QEMU coding style requires use of typedef:

typedef struct {
    ...
} BlockLimits;

Stefan
diff mbox

Patch

diff --git a/include/block/block_int.h b/include/block/block_int.h
index cf78c3f..cda99f9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -227,6 +227,20 @@  struct BlockDriver {
     QLIST_ENTRY(BlockDriver) list;
 };
 
+struct BlockLimits {
+    /* maximum number of sectors that can be discarded at once */
+    int max_discard;
+
+    /* optimal alignment for discard requests in sectors */
+    int64_t discard_alignment;
+
+    /* maximum number of sectors that can zeroized at once */
+    int max_write_zeroes;
+
+    /* optimal alignment for write zeroes requests in sectors */
+    int64_t write_zeroes_alignment;
+};
+
 /*
  * Note: the function bdrv_append() copies and swaps contents of
  * BlockDriverStates, so if you add new fields to this struct, please
@@ -280,6 +294,9 @@  struct BlockDriverState {
     uint64_t total_time_ns[BDRV_MAX_IOTYPE];
     uint64_t wr_highest_sector;
 
+    /* I/O Limits */
+    struct BlockLimits bl;
+
     /* Whether the disk can expand beyond total_sectors */
     int growable;