Re: [PATCH v4 2/2] hw/block/nvme: add the dataset management command

[Klaus Jensen]

On Oct 22 10:50, Keith Busch wrote:
> On Thu, Oct 22, 2020 at 07:43:33PM +0200, Klaus Jensen wrote:
> > On Oct 22 08:01, Keith Busch wrote:
> > > On Thu, Oct 22, 2020 at 09:33:13AM +0200, Klaus Jensen wrote:
> > > > +        if (--(*discards)) {
> > > > +            status = NVME_NO_COMPLETE;
> > > > +        } else {
> > > > +            g_free(discards);
> > > > +            req->opaque = NULL;
> > > 
> > > This case needs a
> > > 
> > >             status = req->status;
> > > 
> > > So that we get the error set in the callback.
> > > 
> > 
> > There are no cases that result in a non-zero status code here.
> 
> Your callback has a case that sets NVME_INTERNAL_DEV_ERROR status. That
> would get ignored if the final discard reference is dropped from the
> submission side.
> 

Oh. Crap. You are right. Nice catch!

> +static void nvme_aio_discard_cb(void *opaque, int ret)
> +{
> +    NvmeRequest *req = opaque;
> +    int *discards = req->opaque;
> +
> +    trace_pci_nvme_aio_discard_cb(nvme_cid(req));
> +
> +    if (ret) {
> +        req->status = NVME_INTERNAL_DEV_ERROR;
> +        trace_pci_nvme_err_aio(nvme_cid(req), strerror(ret),
> +                               req->status);
> +    }

signature.asc
Description: PGP signature