[PATCH v2 0/5] block: Documentation improvment

[PATCH v2 0/5] block: Documentation improvment

[Philippe Mathieu-Daudé]

These patches contain documentation I wrote down while
looking at the code.

Supersedes: <20200504094641.4963-1-philmd@redhat.com>

Philippe Mathieu-Daudé (5):
  qemu/osdep: Document qemu_memalign() and friends
  qemu/bitmap: Document bitmap_new() returned pointer
  block/block: Document BlockSizes fields
  sysemu/block-backend: Document blk_read()/blk_pwrite()
  sysemu/block-backend: Document blk_get_max_iov() returned value

 include/block/block.h          |  8 ++++++--
 include/qemu/bitmap.h          |  2 ++
 include/qemu/osdep.h           |  3 +++
 include/sysemu/block-backend.h | 27 +++++++++++++++++++++++++++
 4 files changed, 38 insertions(+), 2 deletions(-)

-- 
2.21.3

[PATCH v2 1/5] qemu/osdep: Document qemu_memalign() and friends

[Philippe Mathieu-Daudé]

Document allocator functions that require a specific
de-allocator call.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/block/block.h          | 4 ++++
 include/qemu/osdep.h           | 3 +++
 include/sysemu/block-backend.h | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index 4de8d8f8a6..480e6b6837 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -525,9 +525,13 @@ void bdrv_img_create(const char *filename, const char *fmt,
 size_t bdrv_min_mem_align(BlockDriverState *bs);
 /* Returns optimal alignment in bytes for bounce buffer */
 size_t bdrv_opt_mem_align(BlockDriverState *bs);
+/* callers must free the returned pointer with qemu_vfree() */
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
+/* callers must free the returned pointer with qemu_vfree() */
 void *qemu_blockalign0(BlockDriverState *bs, size_t size);
+/* callers must free the returned pointer with qemu_vfree() */
 void *qemu_try_blockalign(BlockDriverState *bs, size_t size);
+/* callers must free the returned pointer with qemu_vfree() */
 void *qemu_try_blockalign0(BlockDriverState *bs, size_t size);
 bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
 
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ff7c17b857..b3eccce762 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -294,8 +294,11 @@ extern int daemon(int, int);
 #endif
 
 int qemu_daemon(int nochdir, int noclose);
+/* callers must free the returned pointer with qemu_vfree() */
 void *qemu_try_memalign(size_t alignment, size_t size);
+/* callers must free the returned pointer with qemu_vfree() */
 void *qemu_memalign(size_t alignment, size_t size);
+/* callers must free the returned pointer with qemu_anon_ram_free() */
 void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared);
 void qemu_vfree(void *ptr);
 void qemu_anon_ram_free(void *ptr, size_t size);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 0917663d89..7996cb61bb 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -205,7 +205,9 @@ uint32_t blk_get_request_alignment(BlockBackend *blk);
 uint32_t blk_get_max_transfer(BlockBackend *blk);
 int blk_get_max_iov(BlockBackend *blk);
 void blk_set_guest_block_size(BlockBackend *blk, int align);
+/* callers must free the returned pointer with qemu_vfree() */
 void *blk_try_blockalign(BlockBackend *blk, size_t size);
+/* callers must free the returned pointer with qemu_vfree() */
 void *blk_blockalign(BlockBackend *blk, size_t size);
 bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp);
 void blk_op_unblock(BlockBackend *blk, BlockOpType op, Error *reason);
-- 
2.21.3

[PATCH v2 2/5] qemu/bitmap: Document bitmap_new() returned pointer

[Philippe Mathieu-Daudé]

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/bitmap.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 82a1d2f41f..0b390ff576 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -90,12 +90,14 @@ int slow_bitmap_intersects(const unsigned long *bitmap1,
                            const unsigned long *bitmap2, long bits);
 long slow_bitmap_count_one(const unsigned long *bitmap, long nbits);
 
+/* callers must free the returned pointer with g_free() */
 static inline unsigned long *bitmap_try_new(long nbits)
 {
     long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
     return g_try_malloc0(len);
 }
 
+/* callers must free the returned pointer with g_free() */
 static inline unsigned long *bitmap_new(long nbits)
 {
     unsigned long *ptr = bitmap_try_new(nbits);
-- 
2.21.3

[PATCH v2 3/5] block/block: Document BlockSizes fields

[Philippe Mathieu-Daudé]

As it is not obvious for a block neophyte what means
the 'log' value, document it.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/block/block.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 480e6b6837..6836876da1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -86,8 +86,8 @@ typedef enum {
 } BdrvRequestFlags;
 
 typedef struct BlockSizes {
-    uint32_t phys;
-    uint32_t log;
+    uint32_t phys;  /* physical block size */
+    uint32_t log;   /* logical block size */
 } BlockSizes;
 
 typedef struct HDGeometry {
-- 
2.21.3

[PATCH v2 4/5] sysemu/block-backend: Document blk_read()/blk_pwrite()

[Philippe Mathieu-Daudé]

The blk_read()/blk_pwrite() return value is not obvious,
document it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: 's/ - /: /' (stefanha)
---
 include/sysemu/block-backend.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 7996cb61bb..b693dfb8f0 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -155,7 +155,31 @@ BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                                   int bytes, BdrvRequestFlags flags,
                                   BlockCompletionFunc *cb, void *opaque);
 int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags);
+
+/**
+ * blk_pread:
+ *
+ * @blk: the block backend where the buffer content is going to be read from
+ * @offset: position in bytes to read at
+ * @buf: the data buffer
+ * @bytes: number of bytes to read
+ *
+ * Returns: the number of bytes read on success, or a negative errno otherwise.
+ */
 int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes);
+
+/**
+ * blk_pwrite:
+ *
+ * @blk: the block backend where the buffer content is going to be written to
+ * @offset: position in bytes to write at
+ * @buf: the data buffer
+ * @bytes: number of bytes of @buf to write
+ * @flags: request flags
+ *
+ * Returns: the number of bytes consumed on success,
+ *          or a negative errno otherwise.
+ */
 int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
                BdrvRequestFlags flags);
 int64_t blk_getlength(BlockBackend *blk);
-- 
2.21.3

Re: [PATCH v2 4/5] sysemu/block-backend: Document blk_read()/blk_pwrite()

[Eric Blake]

On 5/15/20 4:19 AM, Philippe Mathieu-Daudé wrote:
> The blk_read()/blk_pwrite() return value is not obvious,

s/read/pread/ here and in the subject

> document it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

> +/**
> + * blk_pread:
> + *
> + * @blk: the block backend where the buffer content is going to be read from
> + * @offset: position in bytes to read at
> + * @buf: the data buffer
> + * @bytes: number of bytes to read
> + *
> + * Returns: the number of bytes read on success, or a negative errno otherwise.
> + */
>   int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes);

"the number of bytes read" is ambiguous - it sounds too much like the 
read() syscall where short reads are successful.  But blk_pread() never 
does short reads, on success, the result is exactly 'bytes'.

> +
> +/**
> + * blk_pwrite:
> + *
> + * @blk: the block backend where the buffer content is going to be written to
> + * @offset: position in bytes to write at
> + * @buf: the data buffer
> + * @bytes: number of bytes of @buf to write
> + * @flags: request flags
> + *
> + * Returns: the number of bytes consumed on success,

Ditto - we don't support short writes, so on success, it is always 'bytes'.

> + *          or a negative errno otherwise.
> + */
>   int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
>                  BdrvRequestFlags flags);
>   int64_t blk_getlength(BlockBackend *blk);
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PATCH v2 5/5] sysemu/block-backend: Document blk_get_max_iov() returned value

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/sysemu/block-backend.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index b693dfb8f0..7b12563c65 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -227,6 +227,7 @@ void blk_eject(BlockBackend *blk, bool eject_flag);
 int blk_get_flags(BlockBackend *blk);
 uint32_t blk_get_request_alignment(BlockBackend *blk);
 uint32_t blk_get_max_transfer(BlockBackend *blk);
+/* Returns the maximum number of iovec elements */
 int blk_get_max_iov(BlockBackend *blk);
 void blk_set_guest_block_size(BlockBackend *blk, int align);
 /* callers must free the returned pointer with qemu_vfree() */
-- 
2.21.3