* [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
@ 2013-08-16 14:15 Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 1/7] add skeleton for BSD licensed "raw" BlockDriver Laszlo Ersek
` (8 more replies)
0 siblings, 9 replies; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
Paolo asked me to write such a driver based on his textual specification
alone. The first patch captures his email in full, the rest re-quotes
parts that are being implemented.
The tree compiles at each patch. The series passes "make check-block".
"block/raw.c" is not removed because I wanted to keep it out of my
series and out of my brain.
Disclaimer: I couldn't care less if the raw block driver was public
domain or AGPLv3+, as long as it qualifies as free software. I'm only
trying to do what Paolo asked of me.
Laszlo Ersek (7):
add skeleton for BSD licensed "raw" BlockDriver
raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev()
raw_bsd: add raw_create()
raw_bsd: introduce "special members"
raw_bsd: add raw_create_options
raw_bsd: register bdrv_raw
switch raw block driver from "raw.o" to "raw_bsd.o"
block/Makefile.objs | 2 +-
block/raw_bsd.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 187 insertions(+), 1 deletions(-)
create mode 100644 block/raw_bsd.c
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 1/7] add skeleton for BSD licensed "raw" BlockDriver
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
@ 2013-08-16 14:15 ` Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 2/7] raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev() Laszlo Ersek
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
On 08/05/13 15:03, Paolo Bonzini wrote:
>
>
> ----- Original Message -----
>> From: "Laszlo Ersek" <lersek@redhat.com>
>> To: "Paolo Bonzini" <pbonzini@redhat.com>
>> Sent: Monday, August 5, 2013 2:43:46 PM
>> Subject: Re: [PATCH 1/2] raw: add license header
>>
>> On 08/02/13 00:27, Paolo Bonzini wrote:
>>> On 08/01/2013 10:13 AM, Christoph Hellwig wrote:
>>>> On Wed, Jul 31, 2013 at 08:19:51AM +0200, Paolo Bonzini wrote:
>>>>> Most of the block layer is under the BSD license, thus it is
>>>>> reasonable to license block/raw.c the same way. CCed people should
>>>>> ACK by replying with a Signed-off-by line.
>>>>
>>>> The coded was intended to be GPLv2.
>>>
>>> Laszlo, would you be willing to do clean-room reverse engineering?
>>>
>>> (No rants, please. :))
>>
>> What's the scope exactly?
>
> It's quite small, it's a file full of forwarders like
>
> static void raw_foo(BlockDriverState *bs)
> {
> return bdrv_foo(bs->file);
> }
>
> It's 170 lines of code, all as boring as this. I only picked you
> because I'm quite certain you have never seen the file (and the answer
> confirmed it).
>
> Basically:
>
> 1) BlockDriver is a struct in which these function members are
> interesting:
>
> .bdrv_reopen_prepare
> .bdrv_co_readv
> .bdrv_co_writev
> .bdrv_co_is_allocated
> .bdrv_co_write_zeroes
> .bdrv_co_discard
> .bdrv_getlength
> .bdrv_get_info
> .bdrv_truncate
> .bdrv_is_inserted
> .bdrv_media_changed
> .bdrv_eject
> .bdrv_lock_medium
> .bdrv_ioctl
> .bdrv_aio_ioctl
> .bdrv_has_zero_init
>
> They should be implemented as simple forwarders (see above).
> There are 16 functions listed here, you can easily see how this
> already accounts for 100+ SLOC roughly...
>
> The implementations of bdrv_co_readv and bdrv_co_writev should also
> call BLKDBG_EVENT on bs->file too, before forwarding to bs->file. The
> events to be generated are BLKDBG_READ_AIO and BLKDBG_WRITE_AIO.
>
> 2) This is also a simple forwarder function:
>
> .bdrv_create
>
> but there is no BlockDriverState argument so the forwarded-to function
> does not have a bs->file argument either. The forwarded-to function
> is bdrv_create_file.
>
> 3) These members are special
>
> .format_name is the string "raw"
> .bdrv_open raw_open should set bs->sg to bs->file->sg and return 0
> .bdrv_close raw_close should do nothing
> .bdrv_probe raw_probe should just return 1.
>
> 4) There is another member, .create_options, which is an array of
> QEMUOptionParameter structs, terminated by an all-zero item. The only
> option you need is for the virtual disk size. You will find something
> to copy from in other block drivers, for example block/qcow2.c.
>
> 5) Formats are registered with bdrv_register (takes a BlockDriver*).
> You also need to pass the caller of bdrv_register to block_init.
>
> 6) I'm not sure how to organize the patch series, so I'll leave this to
> your creativity. I guess in this case move/copy detection of git should
> be disabled. I would definitely include this spec in the commit
> message as a proof of clean-room reverse engineering.
>
> 7) Remember a BSD header like the one in block.c.
>
> Paolo
This patch implements the email up to the paragraph ending with "100+ SLOC
roughly". The skeleton is generated from the list there, with a simple
shell loop using "sed" and the raw_foo() template.
The BSD license block is copied (and reflowed) from
"util/qemu-progress.c".
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
block/raw_bsd.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 108 insertions(+), 0 deletions(-)
create mode 100644 block/raw_bsd.c
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
new file mode 100644
index 0000000..5c17d53
--- /dev/null
+++ b/block/raw_bsd.c
@@ -0,0 +1,108 @@
+/* BlockDriver implementation for "raw"
+ *
+ * Copyright (C) 2013, Red Hat, Inc.
+ *
+ * Author:
+ * Laszlo Ersek <lersek@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "block/block_int.h"
+
+static TYPE raw_reopen_prepare(BlockDriverState *bs)
+{
+ return bdrv_reopen_prepare(bs->file);
+}
+
+static TYPE raw_co_readv(BlockDriverState *bs)
+{
+ return bdrv_co_readv(bs->file);
+}
+
+static TYPE raw_co_writev(BlockDriverState *bs)
+{
+ return bdrv_co_writev(bs->file);
+}
+
+static TYPE raw_co_is_allocated(BlockDriverState *bs)
+{
+ return bdrv_co_is_allocated(bs->file);
+}
+
+static TYPE raw_co_write_zeroes(BlockDriverState *bs)
+{
+ return bdrv_co_write_zeroes(bs->file);
+}
+
+static TYPE raw_co_discard(BlockDriverState *bs)
+{
+ return bdrv_co_discard(bs->file);
+}
+
+static TYPE raw_getlength(BlockDriverState *bs)
+{
+ return bdrv_getlength(bs->file);
+}
+
+static TYPE raw_get_info(BlockDriverState *bs)
+{
+ return bdrv_get_info(bs->file);
+}
+
+static TYPE raw_truncate(BlockDriverState *bs)
+{
+ return bdrv_truncate(bs->file);
+}
+
+static TYPE raw_is_inserted(BlockDriverState *bs)
+{
+ return bdrv_is_inserted(bs->file);
+}
+
+static TYPE raw_media_changed(BlockDriverState *bs)
+{
+ return bdrv_media_changed(bs->file);
+}
+
+static TYPE raw_eject(BlockDriverState *bs)
+{
+ return bdrv_eject(bs->file);
+}
+
+static TYPE raw_lock_medium(BlockDriverState *bs)
+{
+ return bdrv_lock_medium(bs->file);
+}
+
+static TYPE raw_ioctl(BlockDriverState *bs)
+{
+ return bdrv_ioctl(bs->file);
+}
+
+static TYPE raw_aio_ioctl(BlockDriverState *bs)
+{
+ return bdrv_aio_ioctl(bs->file);
+}
+
+static TYPE raw_has_zero_init(BlockDriverState *bs)
+{
+ return bdrv_has_zero_init(bs->file);
+}
+
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 2/7] raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev()
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 1/7] add skeleton for BSD licensed "raw" BlockDriver Laszlo Ersek
@ 2013-08-16 14:15 ` Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 3/7] raw_bsd: add raw_create() Laszlo Ersek
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
On 08/05/13 15:03, Paolo Bonzini wrote:
>
> [...]
>
> 1) BlockDriver is a struct in which these function members are
> interesting:
>
> .bdrv_reopen_prepare
> .bdrv_co_readv
> .bdrv_co_writev
> .bdrv_co_is_allocated
> .bdrv_co_write_zeroes
> .bdrv_co_discard
> .bdrv_getlength
> .bdrv_get_info
> .bdrv_truncate
> .bdrv_is_inserted
> .bdrv_media_changed
> .bdrv_eject
> .bdrv_lock_medium
> .bdrv_ioctl
> .bdrv_aio_ioctl
> .bdrv_has_zero_init
>
> They should be implemented as simple forwarders (see above). There are
> 16 functions listed here, you can easily see how this already accounts
> for 100+ SLOC roughly...
>
> The implementations of bdrv_co_readv and bdrv_co_writev should also call
> BLKDBG_EVENT on bs->file too, before forwarding to bs->file. The events
> to be generated are BLKDBG_READ_AIO and BLKDBG_WRITE_AIO.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
block/raw_bsd.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 5c17d53..19091a3 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -33,11 +33,13 @@ static TYPE raw_reopen_prepare(BlockDriverState *bs)
static TYPE raw_co_readv(BlockDriverState *bs)
{
+ BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
return bdrv_co_readv(bs->file);
}
static TYPE raw_co_writev(BlockDriverState *bs)
{
+ BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
return bdrv_co_writev(bs->file);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 3/7] raw_bsd: add raw_create()
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 1/7] add skeleton for BSD licensed "raw" BlockDriver Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 2/7] raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev() Laszlo Ersek
@ 2013-08-16 14:15 ` Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 4/7] raw_bsd: introduce "special members" Laszlo Ersek
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
On 08/05/13 15:03, Paolo Bonzini wrote:
>
> [...]
>
> 2) This is also a simple forwarder function:
>
> .bdrv_create
>
> but there is no BlockDriverState argument so the forwarded-to function
> does not have a bs->file argument either. The forwarded-to function is
> bdrv_create_file.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
block/raw_bsd.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 19091a3..5bcbe71 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -108,3 +108,7 @@ static TYPE raw_has_zero_init(BlockDriverState *bs)
return bdrv_has_zero_init(bs->file);
}
+static TYPE raw_create(void)
+{
+ return bdrv_create_file();
+}
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 4/7] raw_bsd: introduce "special members"
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
` (2 preceding siblings ...)
2013-08-16 14:15 ` [Qemu-devel] [PATCH 3/7] raw_bsd: add raw_create() Laszlo Ersek
@ 2013-08-16 14:15 ` Laszlo Ersek
2013-08-20 8:11 ` Kevin Wolf
2013-08-16 14:15 ` [Qemu-devel] [PATCH 5/7] raw_bsd: add raw_create_options Laszlo Ersek
` (4 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
On 08/05/13 15:03, Paolo Bonzini wrote:
>
> [...]
>
> 3) These members are special
>
> .format_name is the string "raw"
> .bdrv_open raw_open should set bs->sg to bs->file->sg and return 0
> .bdrv_close raw_close should do nothing
> .bdrv_probe raw_probe should just return 1.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
block/raw_bsd.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 5bcbe71..86e018d 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -112,3 +112,23 @@ static TYPE raw_create(void)
{
return bdrv_create_file();
}
+
+static const char *raw_format_name(void)
+{
+ return "raw";
+}
+
+static int raw_open(BlockDriverState *bs)
+{
+ bs->sg = bs->file->sg;
+ return 0;
+}
+
+static void raw_close(void)
+{
+}
+
+static int raw_probe(void)
+{
+ return 1;
+}
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 5/7] raw_bsd: add raw_create_options
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
` (3 preceding siblings ...)
2013-08-16 14:15 ` [Qemu-devel] [PATCH 4/7] raw_bsd: introduce "special members" Laszlo Ersek
@ 2013-08-16 14:15 ` Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 6/7] raw_bsd: register bdrv_raw Laszlo Ersek
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
On 08/05/13 15:03, Paolo Bonzini wrote:
>
> [...]
>
> 4) There is another member, .create_options, which is an array of
> QEMUOptionParameter structs, terminated by an all-zero item. The only
> option you need is for the virtual disk size. You will find something
> to copy from in other block drivers, for example block/qcow2.c.
Code taken and adapted from "block/qcow2.c", as suggested. The code being
copied/modified is blamed on
commit 20d97356c9df6d68fbd37d6334fdb7063f24eab6
Author: Blue Swirl <blauwirbel@gmail.com>
Date: Fri Apr 23 20:19:47 2010 +0000
Fix OpenBSD build
and
commit 7c80ab3f21f0b1342f23057d4345ae266c7348d9
Author: Jes Sorensen <Jes.Sorensen@redhat.com>
Date: Fri Dec 17 16:02:39 2010 +0100
block/qcow2.c: rename qcow_ functions to qcow2_
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
block/raw_bsd.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 86e018d..12debb2 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -1,6 +1,7 @@
/* BlockDriver implementation for "raw"
*
- * Copyright (C) 2013, Red Hat, Inc.
+ * Copyright (C) 2010, 2013, Red Hat, Inc.
+ * Copyright (C) 2010, Blue Swirl <blauwirbel@gmail.com>
*
* Author:
* Laszlo Ersek <lersek@redhat.com>
@@ -25,6 +26,16 @@
*/
#include "block/block_int.h"
+#include "qemu/option.h"
+
+static const QEMUOptionParameter raw_create_options[] = {
+ {
+ .name = BLOCK_OPT_SIZE,
+ .type = OPT_SIZE,
+ .help = "Virtual disk size"
+ },
+ { 0 }
+};
static TYPE raw_reopen_prepare(BlockDriverState *bs)
{
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 6/7] raw_bsd: register bdrv_raw
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
` (4 preceding siblings ...)
2013-08-16 14:15 ` [Qemu-devel] [PATCH 5/7] raw_bsd: add raw_create_options Laszlo Ersek
@ 2013-08-16 14:15 ` Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 7/7] switch raw block driver from "raw.o" to "raw_bsd.o" Laszlo Ersek
` (2 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
On 08/05/13 15:03, Paolo Bonzini wrote:
>
> [...]
>
> 5) Formats are registered with bdrv_register (takes a BlockDriver*). You
> also need to pass the caller of bdrv_register to block_init.
Fill in the BlockDriver structure with the raw_*() functions that have
been added to "block/raw_bsd.c", in the order the fields are defined in
"include/block/block_int.h".
I needed more explanation / naming examples for registering the driver
than what Paolo gave me, so I copied / adapted from "block/qcow2.c". The
parts I took as basis for modification are blamed on
commit 5efa9d5a8b18841c9c62208a494d7f519238979a
Author: Anthony Liguori <aliguori@us.ibm.com>
Date: Sat May 9 17:03:42 2009 -0500
Convert block infrastructure to use new module init functionality
commit 20d97356c9df6d68fbd37d6334fdb7063f24eab6
Author: Blue Swirl <blauwirbel@gmail.com>
Date: Fri Apr 23 20:19:47 2010 +0000
Fix OpenBSD build
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
block/raw_bsd.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 12debb2..79916c4 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -2,6 +2,7 @@
*
* Copyright (C) 2010, 2013, Red Hat, Inc.
* Copyright (C) 2010, Blue Swirl <blauwirbel@gmail.com>
+ * Copyright (C) 2009, Anthony Liguori <aliguori@us.ibm.com>
*
* Author:
* Laszlo Ersek <lersek@redhat.com>
@@ -124,11 +125,6 @@ static TYPE raw_create(void)
return bdrv_create_file();
}
-static const char *raw_format_name(void)
-{
- return "raw";
-}
-
static int raw_open(BlockDriverState *bs)
{
bs->sg = bs->file->sg;
@@ -143,3 +139,35 @@ static int raw_probe(void)
{
return 1;
}
+
+static BlockDriver bdrv_raw = {
+ .format_name = "raw",
+ .bdrv_probe = &raw_probe,
+ .bdrv_reopen_prepare = &raw_reopen_prepare,
+ .bdrv_open = &raw_open,
+ .bdrv_close = &raw_close,
+ .bdrv_create = &raw_create,
+ .bdrv_co_readv = &raw_co_readv,
+ .bdrv_co_writev = &raw_co_writev,
+ .bdrv_co_write_zeroes = &raw_co_write_zeroes,
+ .bdrv_co_discard = &raw_co_discard,
+ .bdrv_co_is_allocated = &raw_co_is_allocated,
+ .bdrv_truncate = &raw_truncate,
+ .bdrv_getlength = &raw_getlength,
+ .bdrv_get_info = &raw_get_info,
+ .bdrv_is_inserted = &raw_is_inserted,
+ .bdrv_media_changed = &raw_media_changed,
+ .bdrv_eject = &raw_eject,
+ .bdrv_lock_medium = &raw_lock_medium,
+ .bdrv_ioctl = &raw_ioctl,
+ .bdrv_aio_ioctl = &raw_aio_ioctl,
+ .create_options = &raw_create_options[0],
+ .bdrv_has_zero_init = &raw_has_zero_init
+};
+
+static void bdrv_raw_init(void)
+{
+ bdrv_register(&bdrv_raw);
+}
+
+block_init(bdrv_raw_init);
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 7/7] switch raw block driver from "raw.o" to "raw_bsd.o"
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
` (5 preceding siblings ...)
2013-08-16 14:15 ` [Qemu-devel] [PATCH 6/7] raw_bsd: register bdrv_raw Laszlo Ersek
@ 2013-08-16 14:15 ` Laszlo Ersek
2013-08-18 14:29 ` Paolo Bonzini
2013-08-16 14:59 ` [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Anthony Liguori
2013-08-20 8:21 ` Kevin Wolf
8 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-16 14:15 UTC (permalink / raw)
To: pbonzini, aliguori, hch, qemu-devel
"Incoming" function prototypes and "outgoing" function calls must match
reality. Implemented using the "struct BlockDriver" definition in
"include/block/block_int.h", and gcc errors & warnings.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
block/Makefile.objs | 2 +-
block/raw_bsd.c | 81 +++++++++++++++++++++++++++++---------------------
2 files changed, 48 insertions(+), 35 deletions(-)
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 4cf9aa4..3bb85b5 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,4 +1,4 @@
-block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
+block-obj-y += raw_bsd.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
block-obj-y += qed-check.o
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 79916c4..07e6f06 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -29,7 +29,7 @@
#include "block/block_int.h"
#include "qemu/option.h"
-static const QEMUOptionParameter raw_create_options[] = {
+static QEMUOptionParameter raw_create_options[] = {
{
.name = BLOCK_OPT_SIZE,
.type = OPT_SIZE,
@@ -38,104 +38,117 @@ static const QEMUOptionParameter raw_create_options[] = {
{ 0 }
};
-static TYPE raw_reopen_prepare(BlockDriverState *bs)
+static int raw_reopen_prepare(BDRVReopenState *reopen_state,
+ BlockReopenQueue *queue, Error **errp)
{
- return bdrv_reopen_prepare(bs->file);
+ BDRVReopenState tmp = *reopen_state;
+
+ tmp.bs = tmp.bs->file;
+ return bdrv_reopen_prepare(&tmp, queue, errp);
}
-static TYPE raw_co_readv(BlockDriverState *bs)
+static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov)
{
BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
- return bdrv_co_readv(bs->file);
+ return bdrv_co_readv(bs->file, sector_num, nb_sectors, qiov);
}
-static TYPE raw_co_writev(BlockDriverState *bs)
+static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov)
{
BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
- return bdrv_co_writev(bs->file);
+ return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
}
-static TYPE raw_co_is_allocated(BlockDriverState *bs)
+static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors,
+ int *pnum)
{
- return bdrv_co_is_allocated(bs->file);
+ return bdrv_co_is_allocated(bs->file, sector_num, nb_sectors, pnum);
}
-static TYPE raw_co_write_zeroes(BlockDriverState *bs)
+static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors)
{
- return bdrv_co_write_zeroes(bs->file);
+ return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors);
}
-static TYPE raw_co_discard(BlockDriverState *bs)
+static int coroutine_fn raw_co_discard(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors)
{
- return bdrv_co_discard(bs->file);
+ return bdrv_co_discard(bs->file, sector_num, nb_sectors);
}
-static TYPE raw_getlength(BlockDriverState *bs)
+static int64_t raw_getlength(BlockDriverState *bs)
{
return bdrv_getlength(bs->file);
}
-static TYPE raw_get_info(BlockDriverState *bs)
+static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
- return bdrv_get_info(bs->file);
+ return bdrv_get_info(bs->file, bdi);
}
-static TYPE raw_truncate(BlockDriverState *bs)
+static int raw_truncate(BlockDriverState *bs, int64_t offset)
{
- return bdrv_truncate(bs->file);
+ return bdrv_truncate(bs->file, offset);
}
-static TYPE raw_is_inserted(BlockDriverState *bs)
+static int raw_is_inserted(BlockDriverState *bs)
{
return bdrv_is_inserted(bs->file);
}
-static TYPE raw_media_changed(BlockDriverState *bs)
+static int raw_media_changed(BlockDriverState *bs)
{
return bdrv_media_changed(bs->file);
}
-static TYPE raw_eject(BlockDriverState *bs)
+static void raw_eject(BlockDriverState *bs, bool eject_flag)
{
- return bdrv_eject(bs->file);
+ bdrv_eject(bs->file, eject_flag);
}
-static TYPE raw_lock_medium(BlockDriverState *bs)
+static void raw_lock_medium(BlockDriverState *bs, bool locked)
{
- return bdrv_lock_medium(bs->file);
+ bdrv_lock_medium(bs->file, locked);
}
-static TYPE raw_ioctl(BlockDriverState *bs)
+static int raw_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
{
- return bdrv_ioctl(bs->file);
+ return bdrv_ioctl(bs->file, req, buf);
}
-static TYPE raw_aio_ioctl(BlockDriverState *bs)
+static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs,
+ unsigned long int req, void *buf,
+ BlockDriverCompletionFunc *cb,
+ void *opaque)
{
- return bdrv_aio_ioctl(bs->file);
+ return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque);
}
-static TYPE raw_has_zero_init(BlockDriverState *bs)
+static int raw_has_zero_init(BlockDriverState *bs)
{
return bdrv_has_zero_init(bs->file);
}
-static TYPE raw_create(void)
+static int raw_create(const char *filename, QEMUOptionParameter *options)
{
- return bdrv_create_file();
+ return bdrv_create_file(filename, options);
}
-static int raw_open(BlockDriverState *bs)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags)
{
bs->sg = bs->file->sg;
return 0;
}
-static void raw_close(void)
+static void raw_close(BlockDriverState *bs)
{
}
-static int raw_probe(void)
+static int raw_probe(const uint8_t *buf, int buf_size, const char *filename)
{
return 1;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
` (6 preceding siblings ...)
2013-08-16 14:15 ` [Qemu-devel] [PATCH 7/7] switch raw block driver from "raw.o" to "raw_bsd.o" Laszlo Ersek
@ 2013-08-16 14:59 ` Anthony Liguori
2013-08-16 15:06 ` Daniel P. Berrange
2013-08-18 14:25 ` Paolo Bonzini
2013-08-20 8:21 ` Kevin Wolf
8 siblings, 2 replies; 18+ messages in thread
From: Anthony Liguori @ 2013-08-16 14:59 UTC (permalink / raw)
To: Laszlo Ersek, pbonzini, hch, qemu-devel
Laszlo Ersek <lersek@redhat.com> writes:
> Paolo asked me to write such a driver based on his textual specification
> alone. The first patch captures his email in full, the rest re-quotes
> parts that are being implemented.
>
> The tree compiles at each patch. The series passes "make check-block".
>
> "block/raw.c" is not removed because I wanted to keep it out of my
> series and out of my brain.
>
> Disclaimer: I couldn't care less if the raw block driver was public
> domain or AGPLv3+, as long as it qualifies as free software. I'm only
> trying to do what Paolo asked of me.
Generally speaking, rewriting parts of QEMU to be !GPL is something I
would strongly, strongly oppose.
I believe that Paolo had a good reason for this though. I suppose the
logic is that we want to expose a "libqemublock" that libvirt can use such
that it can stop parsing qcow2 files.
Now libvirt just needs LGPLv2+, right?
Is the JSON mode of qemu-img info not sufficient for libvirt's purposes?
Is there additional logic behind having a libqemublock?
Regards,
Anthony Liguori
>
> Laszlo Ersek (7):
> add skeleton for BSD licensed "raw" BlockDriver
> raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev()
> raw_bsd: add raw_create()
> raw_bsd: introduce "special members"
> raw_bsd: add raw_create_options
> raw_bsd: register bdrv_raw
> switch raw block driver from "raw.o" to "raw_bsd.o"
>
> block/Makefile.objs | 2 +-
> block/raw_bsd.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 187 insertions(+), 1 deletions(-)
> create mode 100644 block/raw_bsd.c
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
2013-08-16 14:59 ` [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Anthony Liguori
@ 2013-08-16 15:06 ` Daniel P. Berrange
2013-08-18 14:25 ` Paolo Bonzini
1 sibling, 0 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2013-08-16 15:06 UTC (permalink / raw)
To: Anthony Liguori; +Cc: pbonzini, Laszlo Ersek, hch, qemu-devel
On Fri, Aug 16, 2013 at 09:59:07AM -0500, Anthony Liguori wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
>
> > Paolo asked me to write such a driver based on his textual specification
> > alone. The first patch captures his email in full, the rest re-quotes
> > parts that are being implemented.
> >
> > The tree compiles at each patch. The series passes "make check-block".
> >
> > "block/raw.c" is not removed because I wanted to keep it out of my
> > series and out of my brain.
> >
> > Disclaimer: I couldn't care less if the raw block driver was public
> > domain or AGPLv3+, as long as it qualifies as free software. I'm only
> > trying to do what Paolo asked of me.
>
> Generally speaking, rewriting parts of QEMU to be !GPL is something I
> would strongly, strongly oppose.
>
> I believe that Paolo had a good reason for this though. I suppose the
> logic is that we want to expose a "libqemublock" that libvirt can use such
> that it can stop parsing qcow2 files.
>
> Now libvirt just needs LGPLv2+, right?
LGPLv2+ is fine, but regardless of license, libvirt won't use any
libqemublock.so library as long as it links to glib with abort on
OOM behaviour.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
2013-08-16 14:59 ` [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Anthony Liguori
2013-08-16 15:06 ` Daniel P. Berrange
@ 2013-08-18 14:25 ` Paolo Bonzini
2013-08-19 16:24 ` Laszlo Ersek
1 sibling, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2013-08-18 14:25 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Laszlo Ersek, hch, qemu-devel
Il 16/08/2013 16:59, Anthony Liguori ha scritto:
> Laszlo Ersek <lersek@redhat.com> writes:
>
>> Paolo asked me to write such a driver based on his textual specification
>> alone. The first patch captures his email in full, the rest re-quotes
>> parts that are being implemented.
>>
>> The tree compiles at each patch. The series passes "make check-block".
>>
>> "block/raw.c" is not removed because I wanted to keep it out of my
>> series and out of my brain.
>>
>> Disclaimer: I couldn't care less if the raw block driver was public
>> domain or AGPLv3+, as long as it qualifies as free software. I'm only
>> trying to do what Paolo asked of me.
>
> Generally speaking, rewriting parts of QEMU to be !GPL is something I
> would strongly, strongly oppose.
>
> I believe that Paolo had a good reason for this though.
The reason is that Christoph said his original version of block/raw.c
was meant to be GPLv2-only. I don't care if the file is BSD or LGPLv2+,
but most of the block layer is BSD, which is why I went for BSD.
It's been a while since I audited the files that would go into
libqemublock, but I remember the only problematic files were block/raw.c
(unlicensed) and block/vdi.c (GPLv2+). Not having VirtualBox support
wouldn't be a big deal.
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] switch raw block driver from "raw.o" to "raw_bsd.o"
2013-08-16 14:15 ` [Qemu-devel] [PATCH 7/7] switch raw block driver from "raw.o" to "raw_bsd.o" Laszlo Ersek
@ 2013-08-18 14:29 ` Paolo Bonzini
2013-08-20 7:51 ` Kevin Wolf
0 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2013-08-18 14:29 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: aliguori, hch, qemu-devel
Il 16/08/2013 16:15, Laszlo Ersek ha scritto:
> +static int raw_reopen_prepare(BDRVReopenState *reopen_state,
> + BlockReopenQueue *queue, Error **errp)
> {
> - return bdrv_reopen_prepare(bs->file);
> + BDRVReopenState tmp = *reopen_state;
> +
> + tmp.bs = tmp.bs->file;
> + return bdrv_reopen_prepare(&tmp, queue, errp);
> }
This should just return zero, my fault.
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
2013-08-18 14:25 ` Paolo Bonzini
@ 2013-08-19 16:24 ` Laszlo Ersek
0 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-19 16:24 UTC (permalink / raw)
To: Paolo Bonzini, Anthony Liguori, Daniel P. Berrange; +Cc: qemu-devel
On 08/18/13 16:25, Paolo Bonzini wrote:
> Il 16/08/2013 16:59, Anthony Liguori ha scritto:
>> Laszlo Ersek <lersek@redhat.com> writes:
>>
>>> Paolo asked me to write such a driver based on his textual specification
>>> alone. The first patch captures his email in full, the rest re-quotes
>>> parts that are being implemented.
>>>
>>> The tree compiles at each patch. The series passes "make check-block".
>>>
>>> "block/raw.c" is not removed because I wanted to keep it out of my
>>> series and out of my brain.
>>>
>>> Disclaimer: I couldn't care less if the raw block driver was public
>>> domain or AGPLv3+, as long as it qualifies as free software. I'm only
>>> trying to do what Paolo asked of me.
>>
>> Generally speaking, rewriting parts of QEMU to be !GPL is something I
>> would strongly, strongly oppose.
>>
>> I believe that Paolo had a good reason for this though.
>
> The reason is that Christoph said his original version of block/raw.c
> was meant to be GPLv2-only. I don't care if the file is BSD or LGPLv2+,
> but most of the block layer is BSD, which is why I went for BSD.
So, is it OK if, after fixing the small problem in 7/7, I post v2 again
under the BSDL?
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] switch raw block driver from "raw.o" to "raw_bsd.o"
2013-08-18 14:29 ` Paolo Bonzini
@ 2013-08-20 7:51 ` Kevin Wolf
0 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2013-08-20 7:51 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: aliguori, Laszlo Ersek, hch, qemu-devel
Am 18.08.2013 um 16:29 hat Paolo Bonzini geschrieben:
> Il 16/08/2013 16:15, Laszlo Ersek ha scritto:
> > +static int raw_reopen_prepare(BDRVReopenState *reopen_state,
> > + BlockReopenQueue *queue, Error **errp)
> > {
> > - return bdrv_reopen_prepare(bs->file);
> > + BDRVReopenState tmp = *reopen_state;
> > +
> > + tmp.bs = tmp.bs->file;
> > + return bdrv_reopen_prepare(&tmp, queue, errp);
> > }
>
> This should just return zero, my fault.
Which is because bdrv_reopen_queue() already queues bs->file for reopen.
The simple return 0; implementation is shared by all other format drivers
that support reopening images.
Kevin
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] raw_bsd: introduce "special members"
2013-08-16 14:15 ` [Qemu-devel] [PATCH 4/7] raw_bsd: introduce "special members" Laszlo Ersek
@ 2013-08-20 8:11 ` Kevin Wolf
0 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2013-08-20 8:11 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: pbonzini, aliguori, hch, qemu-devel
Am 16.08.2013 um 16:15 hat Laszlo Ersek geschrieben:
> On 08/05/13 15:03, Paolo Bonzini wrote:
> >
> > [...]
> >
> > 3) These members are special
> >
> > .format_name is the string "raw"
> > .bdrv_open raw_open should set bs->sg to bs->file->sg and return 0
> > .bdrv_close raw_close should do nothing
> > .bdrv_probe raw_probe should just return 1.
>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> block/raw_bsd.c | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index 5bcbe71..86e018d 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -112,3 +112,23 @@ static TYPE raw_create(void)
> {
> return bdrv_create_file();
> }
> +
> +static const char *raw_format_name(void)
> +{
> + return "raw";
> +}
> +
> +static int raw_open(BlockDriverState *bs)
> +{
> + bs->sg = bs->file->sg;
I know that Paolo explicitly made this requirement, but I think it is
ugly. We should instead fix bdrv_is_sg() to look at bs->file if it
exists, like many other functions already do, and change the two readers
of bs->sg in block.c to use bdrv_is_sg().
> + return 0;
> +}
> +
> +static void raw_close(void)
> +{
> +}
> +
> +static int raw_probe(void)
> +{
> + return 1;
> +}
Maybe add a comment here like "smallest possible positive score so that
raw is used if and only if no other block driver works".
Kevin
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
` (7 preceding siblings ...)
2013-08-16 14:59 ` [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Anthony Liguori
@ 2013-08-20 8:21 ` Kevin Wolf
2013-08-21 8:20 ` Laszlo Ersek
8 siblings, 1 reply; 18+ messages in thread
From: Kevin Wolf @ 2013-08-20 8:21 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: pbonzini, aliguori, hch, stefanha, qemu-devel
Am 16.08.2013 um 16:15 hat Laszlo Ersek geschrieben:
> Paolo asked me to write such a driver based on his textual specification
> alone. The first patch captures his email in full, the rest re-quotes
> parts that are being implemented.
>
> The tree compiles at each patch. The series passes "make check-block".
>
> "block/raw.c" is not removed because I wanted to keep it out of my
> series and out of my brain.
>
> Disclaimer: I couldn't care less if the raw block driver was public
> domain or AGPLv3+, as long as it qualifies as free software. I'm only
> trying to do what Paolo asked of me.
>
> Laszlo Ersek (7):
> add skeleton for BSD licensed "raw" BlockDriver
> raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev()
> raw_bsd: add raw_create()
> raw_bsd: introduce "special members"
> raw_bsd: add raw_create_options
> raw_bsd: register bdrv_raw
> switch raw block driver from "raw.o" to "raw_bsd.o"
>
> block/Makefile.objs | 2 +-
> block/raw_bsd.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 187 insertions(+), 1 deletions(-)
> create mode 100644 block/raw_bsd.c
Reviewed if the individual added functions make sense, whether all
necessary function from struct BlockDriver are implemented, and which
fields from BlockDriverState need special handling (it's only bs->sg,
and we should probably get rid of that requirement)
Looks good in general, but please CC Stefan and me for v2 (like for all
block patches).
Kevin
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
2013-08-20 8:21 ` Kevin Wolf
@ 2013-08-21 8:20 ` Laszlo Ersek
2013-08-21 9:03 ` Paolo Bonzini
0 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2013-08-21 8:20 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pbonzini, stefanha, hch, Anthony Liguori, qemu-devel
On 08/20/13 10:21, Kevin Wolf wrote:
> Am 16.08.2013 um 16:15 hat Laszlo Ersek geschrieben:
>> Paolo asked me to write such a driver based on his textual specification
>> alone. The first patch captures his email in full, the rest re-quotes
>> parts that are being implemented.
>>
>> The tree compiles at each patch. The series passes "make check-block".
>>
>> "block/raw.c" is not removed because I wanted to keep it out of my
>> series and out of my brain.
>>
>> Disclaimer: I couldn't care less if the raw block driver was public
>> domain or AGPLv3+, as long as it qualifies as free software. I'm only
>> trying to do what Paolo asked of me.
>>
>> Laszlo Ersek (7):
>> add skeleton for BSD licensed "raw" BlockDriver
>> raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev()
>> raw_bsd: add raw_create()
>> raw_bsd: introduce "special members"
>> raw_bsd: add raw_create_options
>> raw_bsd: register bdrv_raw
>> switch raw block driver from "raw.o" to "raw_bsd.o"
>>
>> block/Makefile.objs | 2 +-
>> block/raw_bsd.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 187 insertions(+), 1 deletions(-)
>> create mode 100644 block/raw_bsd.c
>
> Reviewed if the individual added functions make sense, whether all
> necessary function from struct BlockDriver are implemented, and which
> fields from BlockDriverState need special handling (it's only bs->sg,
> and we should probably get rid of that requirement)
>
> Looks good in general, but please CC Stefan and me for v2 (like for all
> block patches).
Thanks for the review.
Regarding your comments for 4/7: can we postpone the bdrv_is_sg() change
to another series?
Because, I can't just rebase / update this series as a "normal" series
-- v2 will still be a "clean room reimplementation", and I must keep
full history (basically, a documentation of the "clean room process") in
the commit log.
So, Paolo's suggestion for 7/7 (ie. raw_reopen_prepare() should just
return 0) will be a separate 8/8, with his email quoted as commit
message. (Normally I would just squash the change and add a short v2
note *outside* the commit log, but that's exactly what we can't do here.)
... Maybe I can still squash the change into 7/7, and extend only the
commit message with Paolo's email, since that includes the wrong v1 code
too.
Anyway, under this MO, I have trouble implementing what you want, if
only from the "how to structure the series" aspect. I think we shouldn't
mix in any changes that are unrelated to the BSDL reimplementation
*purpose* (as opposed to, related to the code).
Once the series is applied, in a minimal, barely working form, under the
BSDL, y'all who actually have a clue about the block layer can very
quickly improve it.
Does that sound acceptable?
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw"
2013-08-21 8:20 ` Laszlo Ersek
@ 2013-08-21 9:03 ` Paolo Bonzini
0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2013-08-21 9:03 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Kevin Wolf, stefanha, hch, Anthony Liguori, qemu-devel
Il 21/08/2013 10:20, Laszlo Ersek ha scritto:
> On 08/20/13 10:21, Kevin Wolf wrote:
>> Am 16.08.2013 um 16:15 hat Laszlo Ersek geschrieben:
>>> Paolo asked me to write such a driver based on his textual specification
>>> alone. The first patch captures his email in full, the rest re-quotes
>>> parts that are being implemented.
>>>
>>> The tree compiles at each patch. The series passes "make check-block".
>>>
>>> "block/raw.c" is not removed because I wanted to keep it out of my
>>> series and out of my brain.
>>>
>>> Disclaimer: I couldn't care less if the raw block driver was public
>>> domain or AGPLv3+, as long as it qualifies as free software. I'm only
>>> trying to do what Paolo asked of me.
>>>
>>> Laszlo Ersek (7):
>>> add skeleton for BSD licensed "raw" BlockDriver
>>> raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev()
>>> raw_bsd: add raw_create()
>>> raw_bsd: introduce "special members"
>>> raw_bsd: add raw_create_options
>>> raw_bsd: register bdrv_raw
>>> switch raw block driver from "raw.o" to "raw_bsd.o"
>>>
>>> block/Makefile.objs | 2 +-
>>> block/raw_bsd.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 187 insertions(+), 1 deletions(-)
>>> create mode 100644 block/raw_bsd.c
>>
>> Reviewed if the individual added functions make sense, whether all
>> necessary function from struct BlockDriver are implemented, and which
>> fields from BlockDriverState need special handling (it's only bs->sg,
>> and we should probably get rid of that requirement)
>>
>> Looks good in general, but please CC Stefan and me for v2 (like for all
>> block patches).
>
> Thanks for the review.
>
> Regarding your comments for 4/7: can we postpone the bdrv_is_sg() change
> to another series?
I think it should be done like that.
> Because, I can't just rebase / update this series as a "normal" series
> -- v2 will still be a "clean room reimplementation", and I must keep
> full history (basically, a documentation of the "clean room process") in
> the commit log.
>
> So, Paolo's suggestion for 7/7 (ie. raw_reopen_prepare() should just
> return 0) will be a separate 8/8, with his email quoted as commit
> message. (Normally I would just squash the change and add a short v2
> note *outside* the commit log, but that's exactly what we can't do here.)
>
> ... Maybe I can still squash the change into 7/7, and extend only the
> commit message with Paolo's email, since that includes the wrong v1 code
> too.
Yes, just squash it.
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-08-21 9:04 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-16 14:15 [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 1/7] add skeleton for BSD licensed "raw" BlockDriver Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 2/7] raw_bsd: emit debug events in bdrv_co_readv() and bdrv_co_writev() Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 3/7] raw_bsd: add raw_create() Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 4/7] raw_bsd: introduce "special members" Laszlo Ersek
2013-08-20 8:11 ` Kevin Wolf
2013-08-16 14:15 ` [Qemu-devel] [PATCH 5/7] raw_bsd: add raw_create_options Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 6/7] raw_bsd: register bdrv_raw Laszlo Ersek
2013-08-16 14:15 ` [Qemu-devel] [PATCH 7/7] switch raw block driver from "raw.o" to "raw_bsd.o" Laszlo Ersek
2013-08-18 14:29 ` Paolo Bonzini
2013-08-20 7:51 ` Kevin Wolf
2013-08-16 14:59 ` [Qemu-devel] [PATCH 0/7] introduce BSD-licensed block driver for "raw" Anthony Liguori
2013-08-16 15:06 ` Daniel P. Berrange
2013-08-18 14:25 ` Paolo Bonzini
2013-08-19 16:24 ` Laszlo Ersek
2013-08-20 8:21 ` Kevin Wolf
2013-08-21 8:20 ` Laszlo Ersek
2013-08-21 9:03 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.