Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
Boris Ostrovsky wrote:
>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?
>
Yes
>
>-boris
>
>>
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
Boris Ostrovsky boris.ostrov...@oracle.com wrote: 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? Yes -boris 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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.
Olaf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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. Olaf -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
---
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", _granularity,
- "discard-alignment", "%u", _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", _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", _granularity,
+ "discard-alignment", "%u", _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", _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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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 o...@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)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen-blkfront: remove type check from blkfront_setup_discard
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
