linux-kernel - Re: [PATCH 2/3] nbd: add support for feature negotiation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAECXXi4YW++_302mcSTX=cPjbguSqcw3Weec85VrLFv1qNHXug@mail.gmail.com>
Date:	Wed, 7 Sep 2011 12:30:26 -0400
From:	Paul Clements <paul.clements@...sios.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
	nbd-general@...ts.sourceforge.net
Subject: Re: [PATCH 2/3] nbd: add support for feature negotiation

Paolo,

thanks for the patch...

On Wed, Sep 7, 2011 at 10:41 AM, Paolo Bonzini <pbonzini@...hat.com> wrote:
> Add a IOCTL to negotiate features between the server and the
> kernel module (mediated by the usermode client).  Features are
> stored in the pre-existing flags field.

I'm not crazy about another ioctl, but we need to get this flag
setting functionality in, and I just have not had the time...

> Also zero out the flags when NBD_DO_IT completes.
>
> Cc: Paul Clements <Paul.Clements@...eleye.com>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
>  drivers/block/nbd.c |    8 ++++++++
>  include/linux/nbd.h |   24 +++++++++++++++---------
>  2 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index be23aec..c4fe9c8 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -85,6 +85,7 @@ static const char *ioctl_cmd_to_ascii(int cmd)
>        case NBD_PRINT_DEBUG: return "print-debug";
>        case NBD_SET_SIZE_BLOCKS: return "set-size-blocks";
>        case NBD_DISCONNECT: return "disconnect";
> +       case NBD_SET_FEATURES: return "set-features";

Could you name this as NBD_SET_FLAGS, as that's more consistent with
what it's really doing?

Otherwise these look good for inclusion.

Thanks,
Paul


>        case BLKROSET: return "set-read-only";
>        case BLKFLSBUF: return "flush-buffer-cache";
>        }
> @@ -634,6 +635,12 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *lo,
>                lo->xmit_timeout = arg * HZ;
>                return 0;
>
> +       case NBD_SET_FEATURES:
> +               if (((u64) arg) & NBD_FEATURE_RESERVED)
> +                       return -EINVAL;
> +               lo->flags = (lo->flags & NBD_FEATURE_RESERVED) | (u64)arg;
> +               return 0;
> +
>        case NBD_SET_SIZE_BLOCKS:
>                lo->bytesize = ((u64) arg) * lo->blksize;
>                bdev->bd_inode->i_size = lo->bytesize;
> @@ -672,6 +679,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *lo,
>                printk(KERN_WARNING "%s: queue cleared\n", lo->disk->disk_name);
>                if (file)
>                        fput(file);
> +               lo->flags = 0;
>                lo->bytesize = 0;
>                bdev->bd_inode->i_size = 0;
>                set_capacity(lo->disk, 0);
> diff --git a/include/linux/nbd.h b/include/linux/nbd.h
> index 0582054..926f19d 100644
> --- a/include/linux/nbd.h
> +++ b/include/linux/nbd.h
> @@ -17,16 +17,22 @@
>
>  #include <linux/types.h>
>
> -#define NBD_SET_SOCK   _IO( 0xab, 0 )
> -#define NBD_SET_BLKSIZE        _IO( 0xab, 1 )
> -#define NBD_SET_SIZE   _IO( 0xab, 2 )
> -#define NBD_DO_IT      _IO( 0xab, 3 )
> -#define NBD_CLEAR_SOCK _IO( 0xab, 4 )
> -#define NBD_CLEAR_QUE  _IO( 0xab, 5 )
> -#define NBD_PRINT_DEBUG        _IO( 0xab, 6 )
> -#define NBD_SET_SIZE_BLOCKS    _IO( 0xab, 7 )
> -#define NBD_DISCONNECT  _IO( 0xab, 8 )
> -#define NBD_SET_TIMEOUT _IO( 0xab, 9 )
> +#define NBD_SET_SOCK           _IO(0xab, 0)
> +#define NBD_SET_BLKSIZE                _IO(0xab, 1)
> +#define NBD_SET_SIZE           _IO(0xab, 2)
> +#define NBD_DO_IT              _IO(0xab, 3)
> +#define NBD_CLEAR_SOCK         _IO(0xab, 4)
> +#define NBD_CLEAR_QUE          _IO(0xab, 5)
> +#define NBD_PRINT_DEBUG                _IO(0xab, 6)
> +#define NBD_SET_SIZE_BLOCKS    _IO(0xab, 7)
> +#define NBD_DISCONNECT         _IO(0xab, 8)
> +#define NBD_SET_TIMEOUT                _IO(0xab, 9)
> +#define NBD_SET_FEATURES       _IO(0xab, 10)
> +
> +/* bits 0 and 1 are already used for flags in the protocol, we do
> + * not want any overlap.  Bits 32-63 are reserved because they
> + * are inaccessible on 32-bit platforms.  */
> +#define NBD_FEATURE_RESERVED   0xFFFFFFFF00000003LL
>
>  enum {
>        NBD_CMD_READ = 0,
> --
> 1.7.6
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/