Re: [PATCH v5 04/11] blksnap: header file of the module interface
From: Thomas Weißschuh <hidden>
Date: 2023-07-17 18:57:41
Also in:
linux-block, linux-fsdevel, lkml
On 2023-06-12 15:52:21+0200, Sergei Shtepa wrote:
[..]
quoted hunk ↗ jump to hunk
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.