From: "Thomas Weißschuh" <thomas@t-8ch.de>
To: Sergei Shtepa <sergei.shtepa@veeam.com>
Cc: axboe@kernel.dk, hch@infradead.org, corbet@lwn.net,
snitzer@kernel.org, viro@zeniv.linux.org.uk, brauner@kernel.org,
dchinner@redhat.com, willy@infradead.org, dlemoal@kernel.org,
jack@suse.cz, ming.lei@redhat.com, linux-block@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org,
Donald Buczek <buczek@molgen.mpg.de>
Subject: Re: [PATCH v5 04/11] blksnap: header file of the module interface
Date: Mon, 17 Jul 2023 20:57:34 +0200 [thread overview]
Message-ID: <822909b0-abd6-4e85-b739-41f8efa6feff@t-8ch.de> (raw)
In-Reply-To: <20230612135228.10702-5-sergei.shtepa@veeam.com>
On 2023-06-12 15:52:21+0200, Sergei Shtepa wrote:
> [..]
> diff --git a/include/uapi/linux/blksnap.h b/include/uapi/linux/blksnap.h
> new file mode 100644
> index 000000000000..2fe3f2a43bc5
> --- /dev/null
> +++ b/include/uapi/linux/blksnap.h
> @@ -0,0 +1,421 @@
> [..]
> +/**
> + * struct blksnap_snapshotinfo - Result for the command
> + * &blkfilter_ctl_blksnap.blkfilter_ctl_blksnap_snapshotinfo.
> + *
> + * @error_code:
> + * Zero if there were no errors while holding the snapshot.
> + * The error code -ENOSPC means that while holding the snapshot, a snapshot
> + * overflow situation has occurred. Other error codes mean other reasons
> + * for failure.
> + * The error code is reset when the device is added to a new snapshot.
> + * @image:
> + * If the snapshot was taken, it stores the block device name of the
> + * image, or empty string otherwise.
> + */
> +struct blksnap_snapshotinfo {
> + __s32 error_code;
> + __u8 image[IMAGE_DISK_NAME_LEN];
Nitpick:
Seems a bit weird to have a signed error code that is always negative.
Couldn't this be an unsigned number or directly return the error from
the ioctl() itself?
> +};
> +
> +/**
> + * DOC: Interface for managing snapshots
> + *
> + * Control commands that are transmitted through the blksnap module interface.
> + */
> +enum blksnap_ioctl {
> + blksnap_ioctl_version,
> + blksnap_ioctl_snapshot_create,
> + blksnap_ioctl_snapshot_destroy,
> + blksnap_ioctl_snapshot_append_storage,
> + blksnap_ioctl_snapshot_take,
> + blksnap_ioctl_snapshot_collect,
> + blksnap_ioctl_snapshot_wait_event,
> +};
> +
> +/**
> + * struct blksnap_version - Module version.
> + *
> + * @major:
> + * Version major part.
> + * @minor:
> + * Version minor part.
> + * @revision:
> + * Revision number.
> + * @build:
> + * Build number. Should be zero.
> + */
> +struct blksnap_version {
> + __u16 major;
> + __u16 minor;
> + __u16 revision;
> + __u16 build;
> +};
> +
> +/**
> + * define IOCTL_BLKSNAP_VERSION - Get module version.
> + *
> + * The version may increase when the API changes. But linking the user space
> + * behavior to the version code does not seem to be a good idea.
> + * To ensure backward compatibility, API changes should be made by adding new
> + * ioctl without changing the behavior of existing ones. The version should be
> + * used for logs.
> + *
> + * Return: 0 if succeeded, negative errno otherwise.
> + */
> +#define IOCTL_BLKSNAP_VERSION \
> + _IOW(BLKSNAP, blksnap_ioctl_version, struct blksnap_version)
Shouldn't this be _IOR()?
"_IOW means userland is writing and kernel is reading. _IOR
means userland is reading and kernel is writing."
The other ioctl definitions seem to need a review, too.
next prev parent reply other threads:[~2023-07-17 18:57 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-12 13:52 [PATCH v5 00/11] blksnap - block devices snapshots module Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 01/11] documentation: Block Device Filtering Mechanism Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 02/11] block: " Sergei Shtepa
2023-07-11 2:02 ` Yu Kuai
2023-07-12 10:04 ` Yu Kuai
2023-07-12 12:34 ` Yu Kuai
2023-07-17 17:39 ` Sergei Shtepa
2023-07-18 1:21 ` Yu Kuai
2023-07-17 16:22 ` Sergei Shtepa
2023-07-17 14:39 ` Sergei Shtepa
2023-07-18 1:37 ` Yu Kuai
2023-07-18 11:25 ` Sergei Shtepa
2023-07-18 12:32 ` Yu Kuai
2023-07-18 16:33 ` Sergei Shtepa
2023-07-19 7:28 ` Yu Kuai
2023-07-19 8:36 ` Sergei Shtepa
2023-07-20 6:14 ` Christoph Hellwig
2023-06-12 13:52 ` [PATCH v5 03/11] documentation: Block Devices Snapshots Module Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 04/11] blksnap: header file of the module interface Sergei Shtepa
2023-06-13 22:25 ` Dave Chinner
2023-06-14 6:26 ` Christoph Hellwig
2023-06-14 9:26 ` Sergei Shtepa
2023-06-14 14:07 ` Christoph Hellwig
2023-06-14 16:43 ` Sergei Shtepa
2023-06-15 0:08 ` Dave Chinner
2023-07-17 18:57 ` Thomas Weißschuh [this message]
2023-07-18 9:53 ` Sergei Shtepa
2023-07-20 6:16 ` Christoph Hellwig
2023-06-12 13:52 ` [PATCH v5 05/11] blksnap: module management interface functions Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 06/11] blksnap: handling and tracking I/O units Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 07/11] blksnap: minimum data storage unit of the original block device Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 08/11] blksnap: difference storage Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 09/11] blksnap: event queue from the " Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 10/11] blksnap: snapshot and snapshot image block device Sergei Shtepa
2023-06-12 13:52 ` [PATCH v5 11/11] blksnap: Kconfig and Makefile Sergei Shtepa
2023-06-12 14:32 ` [PATCH v5 00/11] blksnap - block devices snapshots module Christoph Hellwig
2023-06-12 16:19 ` Eric Biggers
2023-06-13 5:50 ` Christoph Hellwig
2023-06-13 10:12 ` Sergei Shtepa
2023-06-14 17:22 ` Eric Biggers
-- strict thread matches above, loose matches on Subject: below --
2023-06-12 13:21 Sergei Shtepa
2023-06-12 13:21 ` [PATCH v5 04/11] blksnap: header file of the module interface Sergei Shtepa
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=822909b0-abd6-4e85-b739-41f8efa6feff@t-8ch.de \
--to=thomas@t-8ch.de \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=buczek@molgen.mpg.de \
--cc=corbet@lwn.net \
--cc=dchinner@redhat.com \
--cc=dlemoal@kernel.org \
--cc=hch@infradead.org \
--cc=jack@suse.cz \
--cc=linux-block@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.lei@redhat.com \
--cc=sergei.shtepa@veeam.com \
--cc=snitzer@kernel.org \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).