Re: [PATCH v2] virtio_blk: add VIRTIO_BLK_F_LIFETIME feature support

[Alvaro Karsz <alvaro.karsz@solid-run.com>]

> I don't see the connection. virtio would often pass through lifetime
> info from the host. If other devices gain more info then it will make
> sense to add that to virtio, too.

> Depends. If we expect more types, then I think we
> can solve this by passing an array of values.


Good idea!

We could pass something like virtio_blk_lifetime_ioctl struct to userspace:

enum blk_lifetime_type {
         VIRTIO_BLK_LIFETIME_PRE_EOL_TYPE_A_B = 1,
};

struct virtio_blk_lifetime_element {
        void *data;
        enum blk_lifetime_type;
};

struct virtio_blk_lifetime_ioctl {
        struct virtio_blk_lifetime_element elements[];
        u32 elements_num;
};

If just VIRTIO_BLK_F_LIFETIME is negotiated, the array size is 1, and
the element type is VIRTIO_BLK_LIFETIME_PRE_EOL_TYPE_A_B, so the user
will know that this is a virtio_blk_lifetime struct.
This seems general enough to handle future additions and to handle out
of order types (if for example VIRTIO_BLK_LIFETIME is not negotiated
and VIRTIO_BLK_LIFETIME_FUTURE is).

The only downside I can think of is if we add new fields to the struct
virtio_blk_lifetime struct, instead of creating a new, dedicated
struct in the spec.
For example:

struct virtio_blk_lifetime {
  le16 pre_eol_info;
  le16 device_lifetime_est_typ_a;
  le16 device_lifetime_est_typ_b;
  le16 device_lifetime_est_typ_c;  //only if
VIRTIO_BLK_LIFETIME_FUTURE is negotiated
};

In this case, we may need to split it into 2 different structs, and
pass it as 2 elements to userspace.

What do you think?
Should I go ahead and create a new version?


Alvaro
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization