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

[Bob Liu]

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:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/