[Qemu-devel] [PATCH 0/2] less confusing block file names

[Qemu-devel] [PATCH 0/2] less confusing block file names

[Eric Blake]

Based on a suggestion here:
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00350.html

Eric Blake (2):
  block: Rename raw_bsd to raw.c
  block: rename raw-{posix,win32} to file-*.c

 include/block/block_int.h           | 2 +-
 block/{raw-posix.c => file-posix.c} | 0
 block/{raw-win32.c => file-win32.c} | 0
 block/gluster.c                     | 4 ++--
 block/{raw_bsd.c => raw.c}          | 0
 MAINTAINERS                         | 6 +++---
 block/Makefile.objs                 | 6 +++---
 block/trace-events                  | 4 ++--
 configure                           | 2 +-
 9 files changed, 12 insertions(+), 12 deletions(-)
 rename block/{raw-posix.c => file-posix.c} (100%)
 rename block/{raw-win32.c => file-win32.c} (100%)
 rename block/{raw_bsd.c => raw.c} (100%)

-- 
2.7.4

[Qemu-devel] [PATCH 1/2] block: Rename raw_bsd to raw.c

[Eric Blake]

The file has nothing to do with the BSD operating system, but
is rather dealing with the raw data format as a BDS.  Simplify
the name to avoid further confusion.  [Plus I hate typing _ in
file names - the shift key slows things down]

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/{raw_bsd.c => raw.c} | 0
 MAINTAINERS                | 2 +-
 block/Makefile.objs        | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename block/{raw_bsd.c => raw.c} (100%)

diff --git a/block/raw_bsd.c b/block/raw.c
similarity index 100%
rename from block/raw_bsd.c
rename to block/raw.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b01fec0..6f984c3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1677,7 +1677,7 @@ F: block/linux-aio.c
 F: include/block/raw-aio.h
 F: block/raw-posix.c
 F: block/raw-win32.c
-F: block/raw_bsd.c
+F: block/raw.c
 F: block/win32-aio.c

 qcow2
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 67a036a..c10941e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,4 +1,4 @@
-block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o
+block-obj-y += raw.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.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
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/2] block: Rename raw_bsd to raw.c

[Laszlo Ersek]

On 10/20/16 03:11, Eric Blake wrote:
> The file has nothing to do with the BSD operating system,

The name carries "_bsd" because of the license that covers the file.
While the file has nothing to do with the BSD operating system, it is
fully related to the BSD license. Please see commit range

  e1c66c6d82fe^..e5b1d99f5528

So, I recommend to remove the BSD/BDS language from the commit message;
it makes the current name appear as a typo. It was not a typo.

Other than that, I entirely welcome this series; I still get confused
about protocol drivers vs. format drivers. Based on an explanation I got
from Kevin two and a half years ago :), for exactly this set of source
files, "raw_bsd.c" (now "raw.c") is a format driver, while "raw-posix.c"
(now "file-posix.c") is a protocol driver. I think the proposed names
render these roles easier to understand.

Thanks!
Laszlo

> but
> is rather dealing with the raw data format as a BDS.  Simplify
> the name to avoid further confusion.  [Plus I hate typing _ in
> file names - the shift key slows things down]
> 
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  block/{raw_bsd.c => raw.c} | 0
>  MAINTAINERS                | 2 +-
>  block/Makefile.objs        | 2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename block/{raw_bsd.c => raw.c} (100%)
> 
> diff --git a/block/raw_bsd.c b/block/raw.c
> similarity index 100%
> rename from block/raw_bsd.c
> rename to block/raw.c
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b01fec0..6f984c3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1677,7 +1677,7 @@ F: block/linux-aio.c
>  F: include/block/raw-aio.h
>  F: block/raw-posix.c
>  F: block/raw-win32.c
> -F: block/raw_bsd.c
> +F: block/raw.c
>  F: block/win32-aio.c
> 
>  qcow2
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 67a036a..c10941e 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -1,4 +1,4 @@
> -block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o
> +block-obj-y += raw.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.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
> 

[Qemu-devel] [PATCH 2/2] block: rename raw-{posix, win32} to file-*.c

[Eric Blake]

These files deal with the file protocol, not the raw format (the
file protocol is often used with other formats, and the raw
protocol is not forced to use the file format).  Rename things
to make it a bit easier to follow.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/block/block_int.h           | 2 +-
 block/{raw-posix.c => file-posix.c} | 0
 block/{raw-win32.c => file-win32.c} | 0
 block/gluster.c                     | 4 ++--
 MAINTAINERS                         | 4 ++--
 block/Makefile.objs                 | 4 ++--
 block/trace-events                  | 4 ++--
 configure                           | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)
 rename block/{raw-posix.c => file-posix.c} (100%)
 rename block/{raw-win32.c => file-win32.c} (100%)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3e79228..36d26cc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -186,7 +186,7 @@ struct BlockDriver {

     /*
      * Flushes all data that was already written to the OS all the way down to
-     * the disk (for example raw-posix calls fsync()).
+     * the disk (for example file-posix.c calls fsync()).
      */
     int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);

diff --git a/block/raw-posix.c b/block/file-posix.c
similarity index 100%
rename from block/raw-posix.c
rename to block/file-posix.c
diff --git a/block/raw-win32.c b/block/file-win32.c
similarity index 100%
rename from block/raw-win32.c
rename to block/file-win32.c
diff --git a/block/gluster.c b/block/gluster.c
index af76d7d5..257ade5 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1172,7 +1172,7 @@ static int qemu_gluster_has_zero_init(BlockDriverState *bs)
  * If @start is in a trailing hole or beyond EOF, return -ENXIO.
  * If we can't find out, return a negative errno other than -ENXIO.
  *
- * (Shamefully copied from raw-posix.c, only miniscule adaptions.)
+ * (Shamefully copied from file-posix.c, only miniscule adaptions.)
  */
 static int find_allocation(BlockDriverState *bs, off_t start,
                            off_t *data, off_t *hole)
@@ -1262,7 +1262,7 @@ static int find_allocation(BlockDriverState *bs, off_t start,
  * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
  * beyond the end of the disk image it will be clamped.
  *
- * (Based on raw_co_get_block_status() from raw-posix.c.)
+ * (Based on raw_co_get_block_status() from file-posix.c.)
  */
 static int64_t coroutine_fn qemu_gluster_co_get_block_status(
         BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f984c3..950ebea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1675,8 +1675,8 @@ L: qemu-block@nongnu.org
 S: Supported
 F: block/linux-aio.c
 F: include/block/raw-aio.h
-F: block/raw-posix.c
-F: block/raw-win32.c
+F: block/file-posix.c
+F: block/file-win32.c
 F: block/raw.c
 F: block/win32-aio.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index c10941e..3fe8910 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -6,8 +6,8 @@ block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
 block-obj-y += quorum.o
 block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
 block-obj-y += block-backend.o snapshot.o qapi.o
-block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
-block-obj-$(CONFIG_POSIX) += raw-posix.o
+block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
+block-obj-$(CONFIG_POSIX) += file-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 block-obj-y += null.o mirror.o commit.o io.o
 block-obj-y += throttle-groups.o
diff --git a/block/trace-events b/block/trace-events
index 05fa13c..627214b 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -55,8 +55,8 @@ qmp_block_job_complete(void *job) "job %p"
 block_job_cb(void *bs, void *job, int ret) "bs %p job %p ret %d"
 qmp_block_stream(void *bs, void *job) "bs %p job %p"

-# block/raw-win32.c
-# block/raw-posix.c
+# block/file-win32.c
+# block/file-posix.c
 paio_submit_co(int64_t offset, int count, int type) "offset %"PRId64" count %d type %d"
 paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) "acb %p opaque %p offset %"PRId64" count %d type %d"

diff --git a/configure b/configure
index dd9e679..e2b169f 100755
--- a/configure
+++ b/configure
@@ -2727,7 +2727,7 @@ if compile_prog "" "" ; then
 fi

 ##########################################
-# xfsctl() probe, used for raw-posix
+# xfsctl() probe, used for file-posix.c
 if test "$xfs" != "no" ; then
   cat > $TMPC << EOF
 #include <stddef.h>  /* NULL */
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/2] block: rename raw-{posix, win32} to file-*.c

[Laszlo Ersek]

On 10/20/16 03:11, Eric Blake wrote:
> These files deal with the file protocol, not the raw format (the
> file protocol is often used with other formats, and the raw
> protocol

s/raw protocol/raw format/

> is not forced to use the file format).

s/file format/file protocol/

I think.

Thanks!
Laszlo

> Rename things
> to make it a bit easier to follow.
> 
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  include/block/block_int.h           | 2 +-
>  block/{raw-posix.c => file-posix.c} | 0
>  block/{raw-win32.c => file-win32.c} | 0
>  block/gluster.c                     | 4 ++--
>  MAINTAINERS                         | 4 ++--
>  block/Makefile.objs                 | 4 ++--
>  block/trace-events                  | 4 ++--
>  configure                           | 2 +-
>  8 files changed, 10 insertions(+), 10 deletions(-)
>  rename block/{raw-posix.c => file-posix.c} (100%)
>  rename block/{raw-win32.c => file-win32.c} (100%)
> 
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 3e79228..36d26cc 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -186,7 +186,7 @@ struct BlockDriver {
> 
>      /*
>       * Flushes all data that was already written to the OS all the way down to
> -     * the disk (for example raw-posix calls fsync()).
> +     * the disk (for example file-posix.c calls fsync()).
>       */
>      int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
> 
> diff --git a/block/raw-posix.c b/block/file-posix.c
> similarity index 100%
> rename from block/raw-posix.c
> rename to block/file-posix.c
> diff --git a/block/raw-win32.c b/block/file-win32.c
> similarity index 100%
> rename from block/raw-win32.c
> rename to block/file-win32.c
> diff --git a/block/gluster.c b/block/gluster.c
> index af76d7d5..257ade5 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -1172,7 +1172,7 @@ static int qemu_gluster_has_zero_init(BlockDriverState *bs)
>   * If @start is in a trailing hole or beyond EOF, return -ENXIO.
>   * If we can't find out, return a negative errno other than -ENXIO.
>   *
> - * (Shamefully copied from raw-posix.c, only miniscule adaptions.)
> + * (Shamefully copied from file-posix.c, only miniscule adaptions.)
>   */
>  static int find_allocation(BlockDriverState *bs, off_t start,
>                             off_t *data, off_t *hole)
> @@ -1262,7 +1262,7 @@ static int find_allocation(BlockDriverState *bs, off_t start,
>   * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
>   * beyond the end of the disk image it will be clamped.
>   *
> - * (Based on raw_co_get_block_status() from raw-posix.c.)
> + * (Based on raw_co_get_block_status() from file-posix.c.)
>   */
>  static int64_t coroutine_fn qemu_gluster_co_get_block_status(
>          BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6f984c3..950ebea 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1675,8 +1675,8 @@ L: qemu-block@nongnu.org
>  S: Supported
>  F: block/linux-aio.c
>  F: include/block/raw-aio.h
> -F: block/raw-posix.c
> -F: block/raw-win32.c
> +F: block/file-posix.c
> +F: block/file-win32.c
>  F: block/raw.c
>  F: block/win32-aio.c
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index c10941e..3fe8910 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -6,8 +6,8 @@ block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
>  block-obj-y += quorum.o
>  block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
>  block-obj-y += block-backend.o snapshot.o qapi.o
> -block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> -block-obj-$(CONFIG_POSIX) += raw-posix.o
> +block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
> +block-obj-$(CONFIG_POSIX) += file-posix.o
>  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>  block-obj-y += null.o mirror.o commit.o io.o
>  block-obj-y += throttle-groups.o
> diff --git a/block/trace-events b/block/trace-events
> index 05fa13c..627214b 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -55,8 +55,8 @@ qmp_block_job_complete(void *job) "job %p"
>  block_job_cb(void *bs, void *job, int ret) "bs %p job %p ret %d"
>  qmp_block_stream(void *bs, void *job) "bs %p job %p"
> 
> -# block/raw-win32.c
> -# block/raw-posix.c
> +# block/file-win32.c
> +# block/file-posix.c
>  paio_submit_co(int64_t offset, int count, int type) "offset %"PRId64" count %d type %d"
>  paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) "acb %p opaque %p offset %"PRId64" count %d type %d"
> 
> diff --git a/configure b/configure
> index dd9e679..e2b169f 100755
> --- a/configure
> +++ b/configure
> @@ -2727,7 +2727,7 @@ if compile_prog "" "" ; then
>  fi
> 
>  ##########################################
> -# xfsctl() probe, used for raw-posix
> +# xfsctl() probe, used for file-posix.c
>  if test "$xfs" != "no" ; then
>    cat > $TMPC << EOF
>  #include <stddef.h>  /* NULL */
> 

Re: [Qemu-devel] [Qemu-block] [PATCH 0/2] less confusing block file names

[John Snow]

On 10/19/2016 09:11 PM, Eric Blake wrote:
> Based on a suggestion here:
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00350.html
>
> Eric Blake (2):
>   block: Rename raw_bsd to raw.c
>   block: rename raw-{posix,win32} to file-*.c
>
>  include/block/block_int.h           | 2 +-
>  block/{raw-posix.c => file-posix.c} | 0
>  block/{raw-win32.c => file-win32.c} | 0
>  block/gluster.c                     | 4 ++--
>  block/{raw_bsd.c => raw.c}          | 0
>  MAINTAINERS                         | 6 +++---
>  block/Makefile.objs                 | 6 +++---
>  block/trace-events                  | 4 ++--
>  configure                           | 2 +-
>  9 files changed, 12 insertions(+), 12 deletions(-)
>  rename block/{raw-posix.c => file-posix.c} (100%)
>  rename block/{raw-win32.c => file-win32.c} (100%)
>  rename block/{raw_bsd.c => raw.c} (100%)
>

You got all the comments and everything!

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH 0/2] less confusing block file names

[Kevin Wolf]

Am 20.10.2016 um 03:11 hat Eric Blake geschrieben:
> Based on a suggestion here:
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00350.html
> 
> Eric Blake (2):
>   block: Rename raw_bsd to raw.c
>   block: rename raw-{posix,win32} to file-*.c

Seems to do what it promises:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>

I'd like to hear a few more opinions on whether it's a good idea, but I
don't think this should stay for too long on the list because we already
have conflicting patches.

One effect that makes me less than fully happy is that 'git log
block/raw.c' without --follow mixes the history of the renamed driver
with the history of the old, differently licensed one. On the other
hand, the removal of the old driver happened three years ago, so it's
probably unlikely that people confuse what belongs to which one and
revive old code accidentally.

Kevin

Re: [Qemu-devel] [PATCH 0/2] less confusing block file names

[Paolo Bonzini]

On 24/10/2016 15:47, Kevin Wolf wrote:
> One effect that makes me less than fully happy is that 'git log
> block/raw.c' without --follow mixes the history of the renamed driver
> with the history of the old, differently licensed one. On the other
> hand, the removal of the old driver happened three years ago, so it's
> probably unlikely that people confuse what belongs to which one and
> revive old code accidentally.

Hmm, this makes it sensible to only apply patch 2, especially since Eric
has corteously split them. :)

Paolo

Re: [Qemu-devel] [PATCH 0/2] less confusing block file names

[Kevin Wolf]

Am 24.10.2016 um 16:44 hat Paolo Bonzini geschrieben:
> On 24/10/2016 15:47, Kevin Wolf wrote:
> > One effect that makes me less than fully happy is that 'git log
> > block/raw.c' without --follow mixes the history of the renamed driver
> > with the history of the old, differently licensed one. On the other
> > hand, the removal of the old driver happened three years ago, so it's
> > probably unlikely that people confuse what belongs to which one and
> > revive old code accidentally.
> 
> Hmm, this makes it sensible to only apply patch 2, especially since Eric
> has corteously split them. :)

Maybe rename to something like raw-format.c instead then in patch 1?

I must admit that it's reasonable enough to make the assumption that
raw_bsd.c has something to do with a BSD-specific block driver.

Kevin

Re: [Qemu-devel] [PATCH 0/2] less confusing block file names

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

On 10/24/2016 10:14 AM, Kevin Wolf wrote:
> Am 24.10.2016 um 16:44 hat Paolo Bonzini geschrieben:
>> On 24/10/2016 15:47, Kevin Wolf wrote:
>>> One effect that makes me less than fully happy is that 'git log
>>> block/raw.c' without --follow mixes the history of the renamed driver
>>> with the history of the old, differently licensed one. On the other
>>> hand, the removal of the old driver happened three years ago, so it's
>>> probably unlikely that people confuse what belongs to which one and
>>> revive old code accidentally.
>>
>> Hmm, this makes it sensible to only apply patch 2, especially since Eric
>> has corteously split them. :)
> 
> Maybe rename to something like raw-format.c instead then in patch 1?

raw-format.c would indeed be a project-unique name; I can respin along
those lines if there is consensus on that front, as well as touch up the
commit message to point out the history of the bsd name (as Laszlo
pointed out, it was the choice of licensing that determined the original
name)

> 
> I must admit that it's reasonable enough to make the assumption that
> raw_bsd.c has something to do with a BSD-specific block driver.
> 
> Kevin
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 0/2] less confusing block file names

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]

On 10/24/2016 08:47 AM, Kevin Wolf wrote:
> Am 20.10.2016 um 03:11 hat Eric Blake geschrieben:
>> Based on a suggestion here:
>> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00350.html
>>
>> Eric Blake (2):
>>   block: Rename raw_bsd to raw.c
>>   block: rename raw-{posix,win32} to file-*.c
> 
> Seems to do what it promises:
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> 
> I'd like to hear a few more opinions on whether it's a good idea, but I
> don't think this should stay for too long on the list because we already
> have conflicting patches.

On the bright side, git with rename detection enabled is generally able
to patch the correct file, even after a rename.

> 
> One effect that makes me less than fully happy is that 'git log
> block/raw.c' without --follow mixes the history of the renamed driver
> with the history of the old, differently licensed one. On the other
> hand, the removal of the old driver happened three years ago, so it's
> probably unlikely that people confuse what belongs to which one and
> revive old code accidentally.

I guess knowing the history explains the current file name; and as you
mentioned, we're starting to get far enough away from the point the file
was created that the history of the name is less relevant and less
likely to cause mistakes in code motion.  And it's not the first time
we've reused a file name within the tree (look at the history of
top-level qjson.c, which has existed twice before being moved to another
location twice).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]