Messages in this thread | | | Date | Fri, 10 Jan 2014 13:07:56 -0500 | From | Boris Ostrovsky <> | Subject | Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard |
| |
On 01/10/2014 11:28 AM, Olaf Hering wrote: > In its initial implementation a check for "type" was added, but only phy > and file are handled. This breaks advertised discard support for other > type values such as qdisk. > > Fix and simplify this function: If the backend advertises discard > support it is supposed to implement it properly, so enable > feature_discard unconditionally. If the backend advertises the need for > a certain granularity and alignment then propagate both properties to > the blocklayer. The discard-secure property is a boolean, update the code > to reflect that. > > Signed-off-by: Olaf Hering <olaf@aepfle.de> > --- > drivers/block/xen-blkfront.c | 40 ++++++++++++++-------------------------- > 1 file changed, 14 insertions(+), 26 deletions(-) > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index c4a4c90..c9e96b9 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -1635,36 +1635,24 @@ blkfront_closing(struct blkfront_info *info) > static void blkfront_setup_discard(struct blkfront_info *info) > { > int err; > - char *type; > unsigned int discard_granularity; > unsigned int discard_alignment; > unsigned int discard_secure; > > - type = xenbus_read(XBT_NIL, info->xbdev->otherend, "type", NULL); > - if (IS_ERR(type)) > - return; > - > - info->feature_secdiscard = 0; > - if (strncmp(type, "phy", 3) == 0) { > - err = xenbus_gather(XBT_NIL, info->xbdev->otherend, > - "discard-granularity", "%u", &discard_granularity, > - "discard-alignment", "%u", &discard_alignment, > - NULL); > - if (!err) { > - info->feature_discard = 1; > - info->discard_granularity = discard_granularity; > - info->discard_alignment = discard_alignment; > - } > - err = xenbus_gather(XBT_NIL, info->xbdev->otherend, > - "discard-secure", "%d", &discard_secure, > - NULL); > - if (!err) > - info->feature_secdiscard = discard_secure; > - > - } else if (strncmp(type, "file", 4) == 0) > - info->feature_discard = 1; > - > - kfree(type); > + info->feature_discard = 1;
If the call below fails, is it safe to continue using discard feature? At the least, are discard_granularity and discard_alignment guaranteed to have sane/safe values?
> + err = xenbus_gather(XBT_NIL, info->xbdev->otherend, > + "discard-granularity", "%u", &discard_granularity, > + "discard-alignment", "%u", &discard_alignment, > + NULL); > + if (!err) { > + info->discard_granularity = discard_granularity; > + info->discard_alignment = discard_alignment; > + } > + err = xenbus_gather(XBT_NIL, info->xbdev->otherend, > + "discard-secure", "%d", &discard_secure, > + NULL); > + if (!err) > + info->feature_secdiscard = !!discard_secure; > }
err variable is not really necessary so you can drop it.
-boris > > static int blkfront_setup_indirect(struct blkfront_info *info)
| |