On 21-01-19 08:32:21, Klaus Jensen wrote:
> On Jan 17 23:53, Minwoo Im wrote:
> > Volatile Write Cache(VWC) feature is set in nvme_ns_setup() in the
> > initial time. This feature is related to block device backed, but this
> > feature is controlled in controller level via Set/Get Features command.
> >
> > This patch removed dependency between nvme and nvme-ns to manage the VWC
> > flag value. Also, it open coded the Get Features for VWC to check all
> > namespaces attached to the controller, and if false detected, return
> > directly false.
> >
> > Signed-off-by: Minwoo Im <minwoo.im....@gmail.com>
> > ---
> > hw/block/nvme-ns.c | 4 ----
> > hw/block/nvme.c | 15 ++++++++++++---
> > hw/block/nvme.h | 1 -
> > 3 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
> > index 32662230130b..c403cd36b6bd 100644
> > --- a/hw/block/nvme-ns.c
> > +++ b/hw/block/nvme-ns.c
> > @@ -89,10 +89,6 @@ static int nvme_ns_init_blk(NvmeCtrl *n, NvmeNamespace
> > *ns, Error **errp)
> > return -1;
> > }
> >
> > - if (blk_enable_write_cache(ns->blkconf.blk)) {
> > - n->features.vwc = 0x1;
> > - }
> > -
> > return 0;
> > }
> >
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index cf0fe28fe6eb..b2a9c48a9d81 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -3033,6 +3033,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n,
> > NvmeRequest *req)
> > NvmeGetFeatureSelect sel = NVME_GETFEAT_SELECT(dw10);
> > uint16_t iv;
> > NvmeNamespace *ns;
> > + int i;
> >
> > static const uint32_t nvme_feature_default[NVME_FID_MAX] = {
> > [NVME_ARBITRATION] = NVME_ARB_AB_NOLIMIT,
> > @@ -3108,7 +3109,17 @@ static uint16_t nvme_get_feature(NvmeCtrl *n,
> > NvmeRequest *req)
> > result = ns->features.err_rec;
> > goto out;
> > case NVME_VOLATILE_WRITE_CACHE:
> > - result = n->features.vwc;
> > + for (i = 1; i <= n->num_namespaces; i++) {
> > + ns = nvme_ns(n, i);
> > + if (!ns) {
> > + continue;
> > + }
> > +
> > + result = blk_enable_write_cache(ns->blkconf.blk);
> > + if (!result) {
> > + break;
> > + }
> > + }
>
> I did a small tweak here and changed `if (!result)` to `if (result)`. We
> want to report that a volatile write cache is present if ANY of the
> namespace backing devices have a write cache.
Oh.... Thanks for the fix!
