From: Kevin Wolf <kwolf@redhat.com>
To: Klaus Birkelund Jensen <klaus@birkelund.eu>
Cc: Keith Busch <keith.busch@intel.com>,
armbru@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org,
Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 0/8] nvme: v1.3, sgls, metadata and new 'ocssd' device
Date: Mon, 20 May 2019 15:01:24 +0200 [thread overview]
Message-ID: <20190520130124.GE5699@localhost.localdomain> (raw)
In-Reply-To: <20190517084234.26923-1-klaus@birkelund.eu>
Am 17.05.2019 um 10:42 hat Klaus Birkelund Jensen geschrieben:
> Hi,
>
> This series of patches contains a number of refactorings to the emulated
> nvme device, adds additional features, such as support for metadata and
> scatter gather lists, and bumps the supported NVMe version to 1.3.
> Lastly, it contains a new 'ocssd' device.
>
> The motivation for the first seven patches is to set everything up for
> the final patch that adds a new 'ocssd' device and associated block
> driver that implements the OpenChannel 2.0 specification[1]. Many of us
> in the OpenChannel comunity have used a qemu fork[2] for emulation of
> OpenChannel devices. The fork is itself based on Keith's qemu-nvme
> tree[3] and we recently merged mainline qemu into it, but the result is
> still a "hybrid" nvme device that supports both conventional nvme and
> the OCSSD 2.0 spec through a 'dialect' mechanism. Merging instead of
> rebasing also created a pretty messy commit history and my efforts to
> try and rebase our work onto mainline was getting hairy to say the
> least. And I was never really happy with the dialect approach anyway.
>
> I have instead prepared this series of fresh patches that incrementally
> adds additional features to the nvme device to bring it into shape for
> finally introducing a new (and separate) 'ocssd' device that emulates an
> OpenChannel 2.0 device by reusing core functionality from the nvme
> device. Providing a separate ocssd device ensures that no ocssd specific
> stuff creeps into the nvme device.
>
> The ocssd device is backed by a new 'ocssd' block driver that holds
> internal meta data and keeps state permanent across power cycles. In the
> future I think we could use the same approach for the nvme device to
> keep internal metadata such as utilization and deallocated blocks.
A backend driver that is specific for a guest device model (i.e. the
device model requires this driver, and the backend is useless without
the device) sounds like a very questionable design.
Metadata like OcssdFormatHeader that is considered part of the image
data, which means that the _actual_ image content without metadata isn't
directly accessible any more feels like a bad idea, too. Simple things
like what a resize operation means (change only the actual disk size as
usual, or is the new size disk + metadata?) become confusing. Attaching
an image to a different device becomes impossible.
The block format driver doesn't seem to actually add much functionality
to a specially crafted raw image: It provides a convenient way to create
such special images and it dumps some values in 'qemu-img info', but the
actual interpretation of the data is left to the device model.
Looking at the options it does provide, my impression is that these
should really be qdev properties, and the place to store them
persistently is something like the libvirt XML. The device doesn't
change any of the values, so there is nothing that QEMU actually needs
to store. What you invented is a one-off way to pass a config file to a
device, but only for one specific device type.
I think this needs to use a much more standard approach to be mergable.
Markus (CCed) as the maintainer for the configuration mechanisms may
have an opinion on this, too.
> For now, the nvme device does not support the Deallocated and
> Unwritten Logical Block Error (DULBE) feature or the Data Set
> Management command as this would require such support.
Doesn't bdrv_co_block_status() provide all the information you need for
that?
Kevin
next prev parent reply other threads:[~2019-05-20 13:11 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-17 8:42 [Qemu-devel] [PATCH 0/8] nvme: v1.3, sgls, metadata and new 'ocssd' device Klaus Birkelund Jensen
2019-05-17 8:42 ` [Qemu-devel] [PATCH 1/8] nvme: move device parameters to separate struct Klaus Birkelund Jensen
2019-05-17 8:42 ` [Qemu-devel] [PATCH 2/8] nvme: bump supported spec to 1.3 Klaus Birkelund Jensen
2019-05-17 8:42 ` [Qemu-devel] [PATCH 3/8] nvme: simplify PRP mappings Klaus Birkelund Jensen
2019-05-17 8:42 ` [Qemu-devel] [PATCH 4/8] nvme: allow multiple i/o's per request Klaus Birkelund Jensen
2019-05-17 8:42 ` [Qemu-devel] [PATCH 5/8] nvme: add support for metadata Klaus Birkelund Jensen
2019-05-22 6:12 ` [Qemu-devel] [Qemu-block] " Klaus Birkelund
2019-05-17 8:42 ` [Qemu-devel] [PATCH 6/8] nvme: add support for scatter gather lists Klaus Birkelund Jensen
2019-05-17 8:42 ` [Qemu-devel] [PATCH 7/8] nvme: keep a copy of the NVMe command in request Klaus Birkelund Jensen
2019-05-17 8:42 ` [Qemu-devel] [PATCH 8/8] nvme: add an OpenChannel 2.0 NVMe device (ocssd) Klaus Birkelund Jensen
2019-05-20 16:45 ` Eric Blake
2019-05-20 17:33 ` Klaus Birkelund
2019-05-20 13:01 ` Kevin Wolf [this message]
2019-05-20 13:32 ` [Qemu-devel] [PATCH 0/8] nvme: v1.3, sgls, metadata and new 'ocssd' device Klaus Birkelund
[not found] ` <20190520193445.GA22742@apples.localdomain>
[not found] ` <20190521080115.GA4971@linux.fritz.box>
2019-05-21 20:14 ` Klaus Birkelund
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=20190520130124.GE5699@localhost.localdomain \
--to=kwolf@redhat.com \
--cc=armbru@redhat.com \
--cc=keith.busch@intel.com \
--cc=klaus@birkelund.eu \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.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).