Re: [Xen-devel] [PATCH 1/2] driver: xen-blkfront: move talk_to_blkback to the correct place

[Bob Liu <bob.liu@xxxxxxxxxx>]

On 05/15/2015 07:14 PM, Roger Pau Monné wrote:
> El 15/05/15 a les 13.03, Bob Liu ha escrit:
>>
>> On 05/15/2015 06:01 PM, Roger Pau Monné wrote:
>>> El 12/05/15 a les 13.01, Bob Liu ha escrit:
>>>> The right place for talk_to_blkback() to query backend features and 
>>>> transport
>>>> parameters is after backend entered XenbusStateInitWait. There is no 
>>>> problem
>>>
>>> talk_to_blkback doesn't gather any backend features, it just publishes
>>> the features supported by the frontend, which AFAICT can be done at any
>>
>> 1) But talk_tlkback will also allocate and initialize the request ring which
>> should be done after backend entered XenbusStateInitWait.
> 
> Maybe setup_blkring should be moved to a more suitable location instead
> of moving the whole function?
> 

Most of other parts in talk_to_blkback() depends on setup_blkring() like write
out ring-ref and event-channel.

Only notify 'feature-persistent' and 'protocol:XEN_IO_PROTO_ABI_NATIVE' can be 
left in front probe().

Then the patch would like this:
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2c61cf8..6b918e0 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1318,17 +1318,6 @@ again:
                message = "writing event-channel";
                goto abort_transaction;
        }
-       err = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
-                           XEN_IO_PROTO_ABI_NATIVE);
-       if (err) {
-               message = "writing protocol";
-               goto abort_transaction;
-       }
-       err = xenbus_printf(xbt, dev->nodename,
-                           "feature-persistent", "%u", 1);
-       if (err)
-               dev_warn(&dev->dev,
-                        "writing persistent grants feature to xenbus");
 
        err = xenbus_transaction_end(xbt, 0);
        if (err) {
@@ -1430,13 +1419,17 @@ static int blkfront_probe(struct xenbus_device *dev,
        info->handle = simple_strtoul(strrchr(dev->nodename, '/')+1, NULL, 0);
        dev_set_drvdata(&dev->dev, info);
 
-       err = talk_to_blkback(dev, info);
+       err = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
+                           XEN_IO_PROTO_ABI_NATIVE);
        if (err) {
-               kfree(info);
-               dev_set_drvdata(&dev->dev, NULL);
-               return err;
+               message = "writing protocol";
+               goto abort_transaction;
        }
-
+       err = xenbus_printf(xbt, dev->nodename,
+                           "feature-persistent", "%u", 1);
+       if (err)
+               dev_warn(&dev->dev,
+                        "writing persistent grants feature to xenbus");
        return 0;
 }
 
@@ -1906,8 +1899,13 @@ static void blkback_changed(struct xenbus_device *dev,
        dev_dbg(&dev->dev, "blkfront:blkback_changed to state %d.\n", 
backend_state);
 
        switch (backend_state) {
-       case XenbusStateInitialising:
        case XenbusStateInitWait:
+               if (talk_to_blkback(dev, info)) {
+                       kfree(info);
+                       dev_set_drvdata(&dev->dev, NULL);
+                       break;
+               }
+       case XenbusStateInitialising:
        case XenbusStateInitialised:
        case XenbusStateReconfiguring:
        case XenbusStateReconfigured:

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel