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

From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alvaro Karsz <alvaro.karsz@solid-run.com>
Cc: Jens Axboe <axboe@kernel.dk>, Paolo Bonzini <pbonzini@redhat.com>,
	Chaitanya Kulkarni <chaitanyak@nvidia.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>
Subject: Re: [PATCH v2] virtio_blk: add VIRTIO_BLK_F_LIFETIME feature support
Date: Sun, 4 Dec 2022 05:59:09 -0500	[thread overview]
Message-ID: <20221204055044-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CAJs=3_Bma3KiOW3Cah2KWiN34TG4c=bdw1bTjB71Fo_QLaKmXw@mail.gmail.com>

On Sun, Dec 04, 2022 at 10:19:38AM +0200, Alvaro Karsz wrote:
> So, we could create a block-general lifetime ioctl with many reserved
> bytes, or create a virtio block specific ioctl without reserved bytes
> at all.

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.

> I think that we should keep it virtio specific, and if a new lifetime
> command is added to the spec with more fields, we could create a new
> ioctl.
> Does Everyone agree?

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

> > I think if you are going to pass struct virtio_blk_lifetime to
> > userspace, better pass it as defined in the spec, in LE format.
> 
> > It's unusual for an ioctl to produce a struct that's not in CPU
> > endianness. I think the kernel should deal with endianness here.
> 
> I'm not sure how to proceed with the endianness matter..
> 
> Alvaro

If it's a generic ioctl then clearly it's native.
For a virtio specific one, we typically use LE and I would be
consistent.

-- 
MST

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