linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: paul.clements@steeleye.com
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] [RESEND] add discard support to nbd
Date: Thu, 30 Aug 2012 15:00:08 -0700	[thread overview]
Message-ID: <20120830150008.c6a5cd37.akpm@linux-foundation.org> (raw)
In-Reply-To: <503e0dcd.6eub+7cESkENGK2l%paul.clements@steeleye.com>


> Subject: [PATCH 1/2] [RESEND] add discard support to nbd

Please don't send multiple patches with the same title.  And please
prefix the patch titles with text which identifies the affected
subsystem.  Documentation/SubmittingPatches goes into details.


On Wed, 29 Aug 2012 08:40:45 -0400
paul.clements@steeleye.com wrote:

> Description: This patch adds a set-flags ioctl, allowing various option
> flags to be set on an nbd device.

That tells us what it does, but omits the all-important "why it does it".  
What's the requirement here?  What value does this change add to NBD users?

What does the interface do and how are users to use it?

Are the nbd ioctls documented anywhere?

> Signed-off-by: Paul Clements <paul.clements@steeleye.com>
> ---
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index d07c9f7..c544bb4 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -78,6 +78,8 @@ static const char *ioctl_cmd_to_ascii(int cmd)
>  	case NBD_SET_SOCK: return "set-sock";
>  	case NBD_SET_BLKSIZE: return "set-blksize";
>  	case NBD_SET_SIZE: return "set-size";
> +	case NBD_SET_TIMEOUT: return "set-timeout";

That was an unchangelogged bugfix.

> +	case NBD_SET_FLAGS: return "set-flags";
>  	case NBD_DO_IT: return "do-it";
>  	case NBD_CLEAR_SOCK: return "clear-sock";
>  	case NBD_CLEAR_QUE: return "clear-que";
> @@ -460,7 +462,7 @@ static void nbd_handle_req(struct nbd_device *nbd, struct request *req)
>  	nbd_cmd(req) = NBD_CMD_READ;
>  	if (rq_data_dir(req) == WRITE) {
>  		nbd_cmd(req) = NBD_CMD_WRITE;
> -		if (nbd->flags & NBD_READ_ONLY) {
> +		if (nbd->flags & NBD_FLAG_READ_ONLY) {
>  			dev_err(disk_to_dev(nbd->disk),
>  				"Write on read-only\n");
>  			goto error_out;
> @@ -642,6 +644,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		nbd->xmit_timeout = arg * HZ;
>  		return 0;
>  
> +	case NBD_SET_FLAGS:
> +		nbd->flags = arg;
> +		return 0;
> +
>  	case NBD_SET_SIZE_BLOCKS:
>  		nbd->bytesize = ((u64) arg) * nbd->blksize;
>  		bdev->bd_inode->i_size = nbd->bytesize;
> diff --git a/include/linux/nbd.h b/include/linux/nbd.h
> index d146ca1..bb349be 100644
> --- a/include/linux/nbd.h
> +++ b/include/linux/nbd.h
> @@ -27,6 +27,7 @@
>  #define NBD_SET_SIZE_BLOCKS	_IO( 0xab, 7 )
>  #define NBD_DISCONNECT  _IO( 0xab, 8 )
>  #define NBD_SET_TIMEOUT _IO( 0xab, 9 )
> +#define NBD_SET_FLAGS   _IO( 0xab, 10)
>  
>  enum {
>  	NBD_CMD_READ = 0,
> @@ -34,6 +35,10 @@ enum {
>  	NBD_CMD_DISC = 2
>  };
>  
> +/* values for flags field */
> +#define NBD_FLAG_HAS_FLAGS	(1 << 0)
> +#define NBD_FLAG_READ_ONLY	(1 << 1)

These could be individually documented right here, at their definition
site.


>  #define nbd_cmd(req) ((req)->cmd[0])
>  
>  /* userspace doesn't need the nbd_device structure */
> @@ -42,10 +47,6 @@ enum {
>  #include <linux/wait.h>
>  #include <linux/mutex.h>
>  
> -/* values for flags field */
> -#define NBD_READ_ONLY 0x0001
> -#define NBD_WRITE_NOCHK 0x0002
> -
>  struct request;
>  
>  struct nbd_device {

      reply	other threads:[~2012-08-30 22:00 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-29 12:40 paul.clements
2012-08-30 22:00 ` Andrew Morton [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120830150008.c6a5cd37.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul.clements@steeleye.com \
    --subject='Re: [PATCH 1/2] [RESEND] add discard support to nbd' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).