[libvirt] [PATCHv2] storage: FS: Tweak some comments and fix typos

--- Notes: Version 2: - tweak most of the messages I'm not going to push this without a review as I'm not a native speaker. src/storage/storage_backend_fs.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 6ebdd46..19add8a 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2013 Red Hat, Inc. + * Copyright (C) 2007-2014 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -460,7 +460,7 @@ cleanup: * @pool storage pool to unmount * * Ensure that a FS storage pool is not mounted on its target location. - * If already unmounted, this is a no-op + * If already unmounted, this is a no-op. * * Returns 0 if successfully unmounted, -1 on error */ @@ -540,9 +540,8 @@ virStorageBackendFileSystemCheck(virConnectPtr conn ATTRIBUTE_UNUSED, * @conn connection to report errors against * @pool storage pool to start * - * Starts a directory or FS based storage pool. - * - * - If it is a FS based pool, mounts the unlying source device on the pool + * Starts a directory or FS based storage pool. If the pool is a FS based + * pool the underlying source device will be mounted. * * Returns 0 on success, -1 on error */ @@ -739,7 +738,7 @@ error: * VIR_STORAGE_POOL_BUILD_OVERWRITE is set, mkfs is always executed, * any existed data on the target device is overwritten unconditionally. * - * - If it is a FS based pool, mounts the unlying source device on the pool + * If the pool is a FS based pool the underlying source device is mounted. * * Returns 0 on success, -1 on error */ @@ -940,12 +939,12 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, /** * @conn connection to report errors against - * @pool storage pool to start + * @pool storage pool to stop * - * Stops a FS based storage pool. + * Stops a FS based storage pool. If @pool is a FS based pool the underlying + * source device is unmounted. All cached data about volumes is released. * - * - If it is a FS based pool, unmounts the unlying source device on the pool - * - Releases all cached data about volumes + * Returns 0 on success, -1 on error. */ #if WITH_STORAGE_FS static int -- 1.8.5.2

On Mon, Jan 13, 2014 at 11:47:20AM +0100, Peter Krempa wrote:
---
Notes: Version 2: - tweak most of the messages
I'm not going to push this without a review as I'm not a native speaker.
Me neither, but I guess 2 proofreaders might be enough in case of reformatting comments.
src/storage/storage_backend_fs.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 6ebdd46..19add8a 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2013 Red Hat, Inc. + * Copyright (C) 2007-2014 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -460,7 +460,7 @@ cleanup: * @pool storage pool to unmount * * Ensure that a FS storage pool is not mounted on its target location. - * If already unmounted, this is a no-op + * If already unmounted, this is a no-op. * * Returns 0 if successfully unmounted, -1 on error */ @@ -540,9 +540,8 @@ virStorageBackendFileSystemCheck(virConnectPtr conn ATTRIBUTE_UNUSED, * @conn connection to report errors against * @pool storage pool to start * - * Starts a directory or FS based storage pool. - * - * - If it is a FS based pool, mounts the unlying source device on the pool + * Starts a directory or FS based storage pool. If the pool is a FS based + * pool the underlying source device will be mounted. *
I guess "s/a FS based pool/FS based,/" sounds a bit better.
* Returns 0 on success, -1 on error */ @@ -739,7 +738,7 @@ error: * VIR_STORAGE_POOL_BUILD_OVERWRITE is set, mkfs is always executed, * any existed data on the target device is overwritten unconditionally. * - * - If it is a FS based pool, mounts the unlying source device on the pool + * If the pool is a FS based pool the underlying source device is mounted. *
Same here.
* Returns 0 on success, -1 on error */ @@ -940,12 +939,12 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED,
/** * @conn connection to report errors against - * @pool storage pool to start + * @pool storage pool to stop * - * Stops a FS based storage pool. + * Stops a FS based storage pool. If @pool is a FS based pool the underlying + * source device is unmounted. All cached data about volumes is released. *
And the same in here.
- * - If it is a FS based pool, unmounts the unlying source device on the pool - * - Releases all cached data about volumes + * Returns 0 on success, -1 on error. */ #if WITH_STORAGE_FS static int -- 1.8.5.2
Since this is just a reformatting of comments (which don't go into any docs), I'd say it's perfectly OK for this to go in for 1.2.1, so ACK from me. Feel free to wait for a native speaker's ACK if you want to, though. Martin

On 01/13/2014 03:47 AM, Peter Krempa wrote:
---
Notes: Version 2: - tweak most of the messages
I'm not going to push this without a review as I'm not a native speaker.
src/storage/storage_backend_fs.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
* - * Starts a directory or FS based storage pool. - * - * - If it is a FS based pool, mounts the unlying source device on the pool + * Starts a directory or FS based storage pool. If the pool is a FS based + * pool the underlying source device will be mounted.
This one is ambiguous: If 'FS' is pronounced 'filesystem', then it is correct as-is. If it is pronounced 'eff-ess', then you want: s/a FS/an FS/ But with some rewording, you can avoid the need for an article, and thus bypass the pronunciation ambiguity: Starts a directory or FS based storage pool. The underlying source device will be mounted for FS based pools.
* * Returns 0 on success, -1 on error */ @@ -739,7 +738,7 @@ error: * VIR_STORAGE_POOL_BUILD_OVERWRITE is set, mkfs is always executed, * any existed data on the target device is overwritten unconditionally. * - * - If it is a FS based pool, mounts the unlying source device on the pool + * If the pool is a FS based pool the underlying source device is mounted.
Same problem, same solution: The underlying source device is mounted for FS based pools.
- * Stops a FS based storage pool. + * Stops a FS based storage pool. If @pool is a FS based pool the underlying + * source device is unmounted. All cached data about volumes is released.
Trickier. Also runs into singular/plural agreement ("all" pairs with "are", "any" with "is"). How about: Stops a file storage pool. The underlying source device is unmounted for FS based pools. Any cached data about volumes is released. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/13/14 17:02, Eric Blake wrote:
On 01/13/2014 03:47 AM, Peter Krempa wrote:
---
Notes: Version 2: - tweak most of the messages
I'm not going to push this without a review as I'm not a native speaker.
src/storage/storage_backend_fs.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
Thanks for the explanation. I went with the version you've suggested and pushed the patch. Peter
participants (3)
-
Eric Blake
-
Martin Kletzander
-
Peter Krempa