Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] xen-blkfront: remove type check from blkfront_setup_discard

0 views
Skip to first unread message

Olaf Hering

unread,
Jan 10, 2014, 11:30:02 AM1/10/14
to
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 <ol...@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;
+ 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;
}

static int blkfront_setup_indirect(struct blkfront_info *info)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Boris Ostrovsky

unread,
Jan 10, 2014, 1:10:01 PM1/10/14
to
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

Olaf Hering

unread,
Jan 10, 2014, 4:40:02 PM1/10/14
to
On Fri, Jan 10, Boris Ostrovsky wrote:

> 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?

Its up to the toolstack to provide sane values. In the worst case
discard fails. In this specific case the three values are optional, so
the calls can fail. I do not know what happens if the backend device
actually needs the values, but the frontend can not send proper discard
requests. Hopefully it will not damage the hardware..

Olaf

Boris Ostrovsky

unread,
Jan 10, 2014, 5:30:01 PM1/10/14
to
On 01/10/2014 04:37 PM, Olaf Hering wrote:
> On Fri, Jan 10, Boris Ostrovsky wrote:
>
>> 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?
> Its up to the toolstack to provide sane values. In the worst case
> discard fails. In this specific case the three values are optional, so
> the calls can fail. I do not know what happens if the backend device
> actually needs the values, but the frontend can not send proper discard
> requests. Hopefully it will not damage the hardware..

I don't know discard code works but it seems to me that if you pass, for
example, zero as discard_granularity (which may happen if
xenbus_gather() fails) then blkdev_issue_discard() in the backend will
set granularity to 1 and continue with discard. This may not be what the
the guest admin requested. And he won't know about this since no error
message is printed anywhere.

Similarly, if xenbug_gather("discard-secure") fails, I think the code
will assume that secure discard has not been requested. I don't know
what security implications this will have but it sounds bad to me.

I think we should at clear feature_discard and print an error in the log
if *either* of xenbus_gather() calls fail.


-boris

Olaf Hering

unread,
Jan 10, 2014, 5:50:01 PM1/10/14
to
On Fri, Jan 10, Boris Ostrovsky wrote:

> I think we should at clear feature_discard and print an error in the log if
> *either* of xenbus_gather() calls fail.

Are you sure about that? AFAIK many other properties are optional as
well. I dont think there is a formal spec about the discard related
properties. Should every backend be required to provide all four
properties?

Olaf

Boris Ostrovsky

unread,
Jan 10, 2014, 6:00:02 PM1/10/14
to
On 01/10/2014 05:49 PM, Olaf Hering wrote:
> On Fri, Jan 10, Boris Ostrovsky wrote:
>
>> I think we should at clear feature_discard and print an error in the log if
>> *either* of xenbus_gather() calls fail.
> Are you sure about that? AFAIK many other properties are optional as
> well. I dont think there is a formal spec about the discard related
> properties. Should every backend be required to provide all four
> properties?

It's not whether the properties are required or not. It's that they may
have been set by the admin but we ignored them. I am particularly
concerned about security setting.

Can you determine from the error whether the call failed or the property
wasn't available?

Alternatively, we may have to require the toolstack that if
feature-discard is provided then all three of these are provided as
well. And then you disable discard on any error.

-boris

Olaf Hering

unread,
Jan 13, 2014, 4:40:02 AM1/13/14
to
On Fri, Jan 10, Boris Ostrovsky wrote:

> I don't know discard code works but it seems to me that if you pass, for
> example, zero as discard_granularity (which may happen if xenbus_gather()
> fails) then blkdev_issue_discard() in the backend will set granularity to 1
> and continue with discard. This may not be what the the guest admin
> requested. And he won't know about this since no error message is printed
> anywhere.

If I understand the code using granularity/alignment correctly, both are
optional properties. So if the granularity is just 1 it means byte
ranges, which is fine if the backend uses FALLOC_FL_PUNCH_HOLE. Also
both properties are not admin controlled, for phy the blkbk drivers just
passes on what it gets from the underlying hardware.

> Similarly, if xenbug_gather("discard-secure") fails, I think the code will
> assume that secure discard has not been requested. I don't know what
> security implications this will have but it sounds bad to me.

There are no security implications, if the backend does not advertise it
then its not present.

After poking around some more it seems that blkif.h is the spec, it does
not say anything that the three properties are optional. Also the
backend drivers in sles11sp2 and mainline create all three properties
unconditionally. So I think a better change is to expect all three
properties in the frontend. I will send another version of the patch.


Olaf

Olaf Hering

unread,
Jan 13, 2014, 5:20:01 AM1/13/14
to
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: remove the check for "type" as the
frontend is not supposed to care about this backend detail. Expect
backends to provide discard-aligment, discard-granularity and
discard-secure properties because interface specification in blkif.h
does not list these properties as optional.

Signed-off-by: Olaf Hering <ol...@aepfle.de>
---
drivers/block/xen-blkfront.c | 33 +++++++++------------------------
1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index c4a4c90..6ef63eb 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1634,37 +1634,22 @@ 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))
+ if (xenbus_gather(XBT_NIL, info->xbdev->otherend,
+ "discard-granularity", "%u", &discard_granularity,
+ "discard-alignment", "%u", &discard_alignment,
+ "discard-secure", "%u", &discard_secure,
+ NULL))
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;
+ info->discard_granularity = discard_granularity;
+ info->discard_alignment = discard_alignment;
+ info->feature_secdiscard = !!discard_secure;

- kfree(type);
+ info->feature_discard = 1;
}

static int blkfront_setup_indirect(struct blkfront_info *info)

Jan Beulich

unread,
Jan 13, 2014, 6:30:02 AM1/13/14
to
>>> On 13.01.14 at 11:14, Olaf Hering <ol...@aepfle.de> wrote:
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -1634,37 +1634,22 @@ 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))
> + if (xenbus_gather(XBT_NIL, info->xbdev->otherend,
> + "discard-granularity", "%u", &discard_granularity,
> + "discard-alignment", "%u", &discard_alignment,
> + "discard-secure", "%u", &discard_secure,
> + NULL))
> return;

You can't do this in one go - the first two and the last one may be
set independently (and are independent in their meaning), and
hence need to be queried independently (xenbus_gather() fails
on the first absent value).

Jan

Olaf Hering

unread,
Jan 13, 2014, 7:10:02 AM1/13/14
to
On Mon, Jan 13, Jan Beulich wrote:

> You can't do this in one go - the first two and the last one may be
> set independently (and are independent in their meaning), and
> hence need to be queried independently (xenbus_gather() fails
> on the first absent value).

Yes, thats the purpose. Since the properties are required its an all or
nothing thing. If they are truly optional then blkif.h should be updated
to say that.

Olaf

Jan Beulich

unread,
Jan 13, 2014, 7:40:01 AM1/13/14
to
>>> On 13.01.14 at 13:01, Olaf Hering <ol...@aepfle.de> wrote:
> On Mon, Jan 13, Jan Beulich wrote:
>
>> You can't do this in one go - the first two and the last one may be
>> set independently (and are independent in their meaning), and
>> hence need to be queried independently (xenbus_gather() fails
>> on the first absent value).
>
> Yes, thats the purpose. Since the properties are required its an all or
> nothing thing. If they are truly optional then blkif.h should be updated
> to say that.

They _are_ optional.

Jan

Olaf Hering

unread,
Jan 13, 2014, 7:50:02 AM1/13/14
to
On Mon, Jan 13, Jan Beulich wrote:

> >>> On 13.01.14 at 13:01, Olaf Hering <ol...@aepfle.de> wrote:
> > On Mon, Jan 13, Jan Beulich wrote:
> >
> >> You can't do this in one go - the first two and the last one may be
> >> set independently (and are independent in their meaning), and
> >> hence need to be queried independently (xenbus_gather() fails
> >> on the first absent value).
> >
> > Yes, thats the purpose. Since the properties are required its an all or
> > nothing thing. If they are truly optional then blkif.h should be updated
> > to say that.
>
> They _are_ optional.

In this case my first patch is correct and should be used.

Olaf

Ian Campbell

unread,
Jan 13, 2014, 8:10:01 AM1/13/14
to
On Mon, 2014-01-13 at 12:34 +0000, Jan Beulich wrote:
> >>> On 13.01.14 at 13:01, Olaf Hering <ol...@aepfle.de> wrote:
> > On Mon, Jan 13, Jan Beulich wrote:
> >
> >> You can't do this in one go - the first two and the last one may be
> >> set independently (and are independent in their meaning), and
> >> hence need to be queried independently (xenbus_gather() fails
> >> on the first absent value).
> >
> > Yes, thats the purpose. Since the properties are required its an all or
> > nothing thing. If they are truly optional then blkif.h should be updated
> > to say that.
>
> They _are_ optional.

But is it true that either they are all present or they are all absent?

Ian.

Jan Beulich

unread,
Jan 13, 2014, 8:20:02 AM1/13/14
to
>>> On 13.01.14 at 14:00, Ian Campbell <Ian.Ca...@citrix.com> wrote:
> On Mon, 2014-01-13 at 12:34 +0000, Jan Beulich wrote:
>> >>> On 13.01.14 at 13:01, Olaf Hering <ol...@aepfle.de> wrote:
>> > On Mon, Jan 13, Jan Beulich wrote:
>> >
>> >> You can't do this in one go - the first two and the last one may be
>> >> set independently (and are independent in their meaning), and
>> >> hence need to be queried independently (xenbus_gather() fails
>> >> on the first absent value).
>> >
>> > Yes, thats the purpose. Since the properties are required its an all or
>> > nothing thing. If they are truly optional then blkif.h should be updated
>> > to say that.
>>
>> They _are_ optional.
>
> But is it true that either they are all present or they are all absent?

No, it's not. discard-secure is independent of the other two (but
those other two are tied together).

Jan

Ian Campbell

unread,
Jan 13, 2014, 8:40:02 AM1/13/14
to
On Mon, 2014-01-13 at 13:16 +0000, Jan Beulich wrote:
> >>> On 13.01.14 at 14:00, Ian Campbell <Ian.Ca...@citrix.com> wrote:
> > On Mon, 2014-01-13 at 12:34 +0000, Jan Beulich wrote:
> >> >>> On 13.01.14 at 13:01, Olaf Hering <ol...@aepfle.de> wrote:
> >> > On Mon, Jan 13, Jan Beulich wrote:
> >> >
> >> >> You can't do this in one go - the first two and the last one may be
> >> >> set independently (and are independent in their meaning), and
> >> >> hence need to be queried independently (xenbus_gather() fails
> >> >> on the first absent value).
> >> >
> >> > Yes, thats the purpose. Since the properties are required its an all or
> >> > nothing thing. If they are truly optional then blkif.h should be updated
> >> > to say that.
> >>
> >> They _are_ optional.
> >
> > But is it true that either they are all present or they are all absent?
>
> No, it's not. discard-secure is independent of the other two (but
> those other two are tied together).

Thanks for clarifying.

David Vrabel

unread,
Jan 13, 2014, 8:50:02 AM1/13/14
to
On 13/01/14 13:16, Jan Beulich wrote:
>>>> On 13.01.14 at 14:00, Ian Campbell <Ian.Ca...@citrix.com> wrote:
>> On Mon, 2014-01-13 at 12:34 +0000, Jan Beulich wrote:
>>>>>> On 13.01.14 at 13:01, Olaf Hering <ol...@aepfle.de> wrote:
>>>> On Mon, Jan 13, Jan Beulich wrote:
>>>>
>>>>> You can't do this in one go - the first two and the last one may be
>>>>> set independently (and are independent in their meaning), and
>>>>> hence need to be queried independently (xenbus_gather() fails
>>>>> on the first absent value).
>>>>
>>>> Yes, thats the purpose. Since the properties are required its an all or
>>>> nothing thing. If they are truly optional then blkif.h should be updated
>>>> to say that.
>>>
>>> They _are_ optional.
>>
>> But is it true that either they are all present or they are all absent?
>
> No, it's not. discard-secure is independent of the other two (but
> those other two are tied together).

Can we have a patch to blkif.h that clarifies this?

e.g.,

feature-discard

...

discard-granularity and discard-offset must also be present if
feature-discard is enabled

discard-secure may also be present if feature-discard is enabled.

David

Jan Beulich

unread,
Jan 13, 2014, 9:00:01 AM1/13/14
to
>>> On 13.01.14 at 14:45, David Vrabel <david....@citrix.com> wrote:
> On 13/01/14 13:16, Jan Beulich wrote:
>>>>> On 13.01.14 at 14:00, Ian Campbell <Ian.Ca...@citrix.com> wrote:
>>> On Mon, 2014-01-13 at 12:34 +0000, Jan Beulich wrote:
>>>>>>> On 13.01.14 at 13:01, Olaf Hering <ol...@aepfle.de> wrote:
>>>>> On Mon, Jan 13, Jan Beulich wrote:
>>>>>
>>>>>> You can't do this in one go - the first two and the last one may be
>>>>>> set independently (and are independent in their meaning), and
>>>>>> hence need to be queried independently (xenbus_gather() fails
>>>>>> on the first absent value).
>>>>>
>>>>> Yes, thats the purpose. Since the properties are required its an all or
>>>>> nothing thing. If they are truly optional then blkif.h should be updated
>>>>> to say that.
>>>>
>>>> They _are_ optional.
>>>
>>> But is it true that either they are all present or they are all absent?
>>
>> No, it's not. discard-secure is independent of the other two (but
>> those other two are tied together).
>
> Can we have a patch to blkif.h that clarifies this?
>
> e.g.,
>
> feature-discard
>
> ...
>
> discard-granularity and discard-offset must also be present if
> feature-discard is enabled

It would be "may" here too afaict. But I'll defer to Konrad, who
has done more work in this area...

Jan

Boris Ostrovsky

unread,
Jan 13, 2014, 10:00:01 AM1/13/14
to
On 01/13/2014 04:30 AM, Olaf Hering wrote:
> On Fri, Jan 10, Boris Ostrovsky wrote:
>
>> I don't know discard code works but it seems to me that if you pass, for
>> example, zero as discard_granularity (which may happen if xenbus_gather()
>> fails) then blkdev_issue_discard() in the backend will set granularity to 1
>> and continue with discard. This may not be what the the guest admin
>> requested. And he won't know about this since no error message is printed
>> anywhere.
> If I understand the code using granularity/alignment correctly, both are
> optional properties. So if the granularity is just 1 it means byte
> ranges, which is fine if the backend uses FALLOC_FL_PUNCH_HOLE. Also
> both properties are not admin controlled, for phy the blkbk drivers just
> passes on what it gets from the underlying hardware.
>
>> Similarly, if xenbug_gather("discard-secure") fails, I think the code will
>> assume that secure discard has not been requested. I don't know what
>> security implications this will have but it sounds bad to me.
> There are no security implications, if the backend does not advertise it
> then its not present.

Right. But my questions was what if the backend does advertise it and
wants the frontent to use it but xenbus_gather() in the frontend fails.
Do we want to silently continue without discard-secure? Is this safe?


-boris

Olaf Hering

unread,
Jan 13, 2014, 6:10:01 PM1/13/14
to
On Mon, Jan 13, Boris Ostrovsky wrote:

> On 01/13/2014 04:30 AM, Olaf Hering wrote:
> >>Similarly, if xenbug_gather("discard-secure") fails, I think the code will
> >>assume that secure discard has not been requested. I don't know what
> >>security implications this will have but it sounds bad to me.
> >There are no security implications, if the backend does not advertise it
> >then its not present.
>
> Right. But my questions was what if the backend does advertise it and wants
> the frontent to use it but xenbus_gather() in the frontend fails. Do we want
> to silently continue without discard-secure? Is this safe?

The frontend can not know that the backend advertised discard-secure
because the frontend just failed to read the property which indicates
discard-secure should be enabled.

Boris Ostrovsky

unread,
Jan 13, 2014, 9:20:02 PM1/13/14
to
On 01/13/2014 06:07 PM, Olaf Hering wrote:
> On Mon, Jan 13, Boris Ostrovsky wrote:
>
>> On 01/13/2014 04:30 AM, Olaf Hering wrote:
>>>> Similarly, if xenbug_gather("discard-secure") fails, I think the code will
>>>> assume that secure discard has not been requested. I don't know what
>>>> security implications this will have but it sounds bad to me.
>>> There are no security implications, if the backend does not advertise it
>>> then its not present.
>> Right. But my questions was what if the backend does advertise it and wants
>> the frontent to use it but xenbus_gather() in the frontend fails. Do we want
>> to silently continue without discard-secure? Is this safe?
> The frontend can not know that the backend advertised discard-secure
> because the frontend just failed to read the property which indicates
> discard-secure should be enabled.

And is it OK for the frontend not to know about this?

I don't understand what the use model for this feature is. Is it just
that the backend advertises its capability and it's up to the frontend
to use it or not -or- is it that the user/admin created the storage with
expectations that it will be used in "secure" manner.

I think if it's the former then losing information about storage
features is OK but if it's the latter then I am not so sure.

Or perhaps it's neither of these two and I am completely missing the
point of this feature.

-boris

David Vrabel

unread,
Jan 14, 2014, 5:50:01 AM1/14/14
to
On 14/01/14 02:11, Boris Ostrovsky wrote:
> On 01/13/2014 06:07 PM, Olaf Hering wrote:
>> On Mon, Jan 13, Boris Ostrovsky wrote:
>>
>>> On 01/13/2014 04:30 AM, Olaf Hering wrote:
>>>>> Similarly, if xenbug_gather("discard-secure") fails, I think the
>>>>> code will
>>>>> assume that secure discard has not been requested. I don't know what
>>>>> security implications this will have but it sounds bad to me.
>>>> There are no security implications, if the backend does not
>>>> advertise it
>>>> then its not present.
>>> Right. But my questions was what if the backend does advertise it and
>>> wants
>>> the frontent to use it but xenbus_gather() in the frontend fails. Do
>>> we want
>>> to silently continue without discard-secure? Is this safe?
>> The frontend can not know that the backend advertised discard-secure
>> because the frontend just failed to read the property which indicates
>> discard-secure should be enabled.
>
> And is it OK for the frontend not to know about this?

Yes.

> I don't understand what the use model for this feature is. Is it just
> that the backend advertises its capability and it's up to the frontend
> to use it or not -or- is it that the user/admin created the storage with
> expectations that it will be used in "secure" manner.

The creator of the data (i.e., the guest) is the one who knows whether
the data is security sensitive. The backend is simply providing the
facility which the guest may or may not make use of.

David

Olaf Hering

unread,
Jan 14, 2014, 10:00:02 AM1/14/14
to
On Mon, Jan 13, David Vrabel wrote:

> Can we have a patch to blkif.h that clarifies this?

What about this change?

diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index 84eb7fd..56e2faa 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -194,6 +194,7 @@
* discard-secure
* Values: 0/1 (boolean)
* Default Value: 0
+ * Notes: 10
*
* A value of "1" indicates that the backend can process BLKIF_OP_DISCARD
* requests with the BLKIF_DISCARD_SECURE flag set.
@@ -323,9 +324,10 @@
* For full interoperability, block front and backends should publish
* identical ring parameters, adjusted for unit differences, to the
* XenStore nodes used in both schemes.
- * (4) Devices that support discard functionality may internally allocate
- * space (discardable extents) in units that are larger than the
- * exported logical block size.
+ * (4) Devices that support discard functionality may internally allocate space
+ * (discardable extents) in units that are larger than the exported logical
+ * block size. The properties discard-granularity and discard-alignment may
+ * be present if the backing device has such requirments.
* (5) The discard-alignment parameter allows a physical device to be
* partitioned into virtual devices that do not necessarily begin or
* end on a discardable extent boundary.
@@ -344,6 +346,8 @@
* grants that can be persistently mapped in the frontend driver, but
* due to the frontent driver implementation it should never be bigger
* than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST.
+ *(10) The discard-secure property may be present and will be set to 1 if the
+ * backing device supports secure discard.
*/

/*


Olaf

David Vrabel

unread,
Jan 14, 2014, 10:20:01 AM1/14/14
to
Clarify that both discard-granularity and discard-alignment must be
present if non-sector-sized granularity is required. e.g.,

"If the backing device has such discardable extents the backend must
provide both discard-granularity and discard-alignment."

You find it useful to add these recommendations:

"Backends supporting discard should include discard-granularity and
discard-alignment even if it supports discarding individual sectors.
Frontends should assume discard-aligment == 0 and discard-granularity ==
sector size if these keys are missing."

> * (5) The discard-alignment parameter allows a physical device to be
> * partitioned into virtual devices that do not necessarily begin or
> * end on a discardable extent boundary.
> @@ -344,6 +346,8 @@
> * grants that can be persistently mapped in the frontend driver, but
> * due to the frontent driver implementation it should never be bigger
> * than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST.
> + *(10) The discard-secure property may be present and will be set to 1 if the
> + * backing device supports secure discard.
> */

David

Konrad Rzeszutek Wilk

unread,
Jan 22, 2014, 7:50:01 PM1/22/14
to
>_______________________________________________
>Xen-devel mailing list
>Xen-...@lists.xen.org
>http://lists.xen.org/xen-devel

It is all 'may'. If there is just 'feature-discard' without any other options that is OK.

Konrad Rzeszutek Wilk

unread,
Jan 22, 2014, 7:50:02 PM1/22/14
to
Yes
0 new messages