>>> On 22.08.11 at 11:19, Li Dongyang <jerry87905@xxxxxxxxx> wrote:
> On Thu, Aug 18, 2011 at 6:18 PM, Jan Beulich <JBeulich@xxxxxxxxxx> wrote:
>> >>> On 18.08.11 at 11:35, Li Dongyang <lidongyang@xxxxxxxxxx> wrote:
>>> @@ -1178,7 +1211,24 @@ static void blkfront_connect(struct blkfront_info
>>> *info)
>>> info->feature_flush = REQ_FLUSH;
>>> info->flush_op = BLKIF_OP_FLUSH_DISKCACHE;
>>> }
>>> -
>>> +
>>> + err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
>>> + "feature-trim", "%d", &trim,
>>> + NULL);
>>
>> So you switched this to use "trim", but ...
>>
>>> +
>>> + if (!err && trim) {
>>> + info->feature_trim = 1;
>>> + err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
>>> + "discard_granularity", "%u", &discard_granularity,
>>> + "discard_alignment", "%u", &discard_alignment,
>>
>> ... you kept these using "discard" - quite inconsistent.
> the discard_granularity and discard_alignment are taken from struct
> queue_limits
> they are needed to setup the queue in guest if we are using a phy
> device has trim.
> so I think we can keep the names here.
The way Linux names them doesn't matter for the xenstore interface.
I think they should be named so that the connection to the base
node's name is obvious.
>>
>> Also, I think (but I may be wrong) that dashes are preferred over
>> underscores in xenstore node names.
> that could be done in xenstore, I used dashes because they are taken
> from struct queue_limits
>>
>>> + NULL);
>>> + if (!err) {
>>> + info->discard_granularity = discard_granularity;
>>> + info->discard_alignment = discard_alignment;
>>
>> I think you should set info->feature_trim only here (and then you can
>> eliminate the local variables).
> those discard_granularity and discard_alignment are only meaningful if
> the backend is a phy device has trim,
> and the 2 args are read from the queue limits from host. if the
> backend is a file, we can go with no discard_granularity
> and discard_alignment because we are about to punch a hole in the image
> file.
Then you should update the condition accordingly. Setting
info->feature_trim prematurely is just calling for later problems.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home