review For Oracle bug 14470382

review For Oracle bug 14470382

[jianhai luan]

orabug: 14470382

When Dom0's network environment change, for example the issue, DomU 
should send send gratuitous ARP initially to notify how to reach it.

To fix the bug, we should modify backend and frontend at the same time. 
For fixing frontend, i filer the new bug 16182568. The review only to 
backend.

For fixing the backend, I have 4 patch for review.

Thanks,
Jason

Re: [PATCH 1/4]review For Oracle bug 14470382

[jianhai luan]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]


On 2013-1-18 9:46, jianhai luan wrote:
> orabug: 14470382
>
> When Dom0's network environment change, for example the issue, DomU 
> should send send gratuitous ARP initially to notify how to reach it.
>
> To fix the bug, we should modify backend and frontend at the same 
> time. For fixing frontend, i filer the new bug 16182568. The review 
> only to backend.
>
> For fixing the backend, I have 4 patch for review.
[PATCH] net/core: add NETDEV_BONDING_FAILOVER event
>
> Thanks,
> Jason



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-net-core-add-NETDEV_BONDING_FAILOVER-event.patch --]
[-- Type: text/plain; charset=gb18030; name="0001-net-core-add-NETDEV_BONDING_FAILOVER-event.patch", Size: 2598 bytes --]

From c1da4ac752b8b0411791d26c678fcf23d2eed242 Mon Sep 17 00:00:00 2001
From: Or Gerlitz <ogerlitz@voltaire.com>
Date: Fri, 13 Jun 2008 18:12:00 -0700
Subject: [PATCH] net/core: add NETDEV_BONDING_FAILOVER event

Add NETDEV_BONDING_FAILOVER event to be used in a successive patch
by bonding to announce fail-over for the active-backup mode through the
netdev events notifier chain mechanism. Such an event can be of use for the
RDMA CM (communication manager) to let native RDMA ULPs (eg NFS-RDMA, iSER)
always be aligned with the IP stack, in the sense that they use the same
ports/links as the stack does. More usages can be done to allow monitoring
tools based on netlink events being aware to bonding fail-over.

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 include/linux/netdevice.h |    1 +
 include/linux/notifier.h  |    1 +
 net/core/dev.c            |    6 ++++++
 3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f27fd20..e92fc83 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1479,6 +1479,7 @@ extern void		__dev_addr_unsync(struct dev_addr_list **to, int *to_count, struct
 extern void		dev_set_promiscuity(struct net_device *dev, int inc);
 extern void		dev_set_allmulti(struct net_device *dev, int inc);
 extern void		netdev_state_change(struct net_device *dev);
+extern void		netdev_bonding_change(struct net_device *dev);
 extern void		netdev_features_change(struct net_device *dev);
 /* Load a device via the kmod */
 extern void		dev_load(struct net *net, const char *name);
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 0ff6224..bd3d72d 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -197,6 +197,7 @@ static inline int notifier_to_errno(int ret)
 #define NETDEV_GOING_DOWN	0x0009
 #define NETDEV_CHANGENAME	0x000A
 #define NETDEV_FEAT_CHANGE	0x000B
+#define NETDEV_BONDING_FAILOVER 0x000C
 
 #define SYS_DOWN	0x0001	/* Notify of system down */
 #define SYS_RESTART	SYS_DOWN
diff --git a/net/core/dev.c b/net/core/dev.c
index 68d8df0..0e45742 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -961,6 +961,12 @@ void netdev_state_change(struct net_device *dev)
 	}
 }
 
+void netdev_bonding_change(struct net_device *dev)
+{
+	call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, dev);
+}
+EXPORT_SYMBOL(netdev_bonding_change);
+
 /**
  *	dev_load 	- load a network module
  *	@net: the applicable net namespace
-- 
1.7.6.5


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 2/4] review For Oracle bug 14470382

[jianhai luan]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]


On 2013-1-18 9:46, jianhai luan wrote:
> orabug: 14470382
>
> When Dom0's network environment change, for example the issue, DomU 
> should send send gratuitous ARP initially to notify how to reach it.
>
> To fix the bug, we should modify backend and frontend at the same 
> time. For fixing frontend, i filer the new bug 16182568. The review 
> only to backend.
>
> For fixing the backend, I have 4 patch for review.
[PATCH] net/core: add NETDEV_BONDING_FAILOVER event
>
> Thanks,
> Jason


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-bonding-deliver-netdev-event-for-fail-over-under-the.patch --]
[-- Type: text/plain; charset=gb18030; name="0002-bonding-deliver-netdev-event-for-fail-over-under-the.patch", Size: 1730 bytes --]

From 01f3109de49a889db8adf9116449727547ee497e Mon Sep 17 00:00:00 2001
From: Or Gerlitz <ogerlitz@voltaire.com>
Date: Fri, 13 Jun 2008 18:12:02 -0700
Subject: [PATCH] bonding: deliver netdev event for fail-over under the
 active-backup mode

under active-backup mode and when there's actual new_active slave,
have bond_change_active_slave() call the networking core to deliver
NETDEV_BONDING_FAILOVER event such that the fail-over can be notable
by code outside of the bonding driver such as the RDMA stack and
monitoring tools.

As the correct context of locking appropriate for notifier calls is RTNL
and nothing else, bond->curr_slave_lock and bond->lock are unlocked and
later locked again. This is ensured by the rest of the code to be safe
under backup-mode AND when new_active is not NULL.

Jay Vosburgh modified the original patch for formatting and fixed a
compiler error.

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/net/bonding/bond_main.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2db2d05..925402b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1203,6 +1203,14 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 				dprintk("delaying gratuitous arp on %s\n",
 					bond->curr_active_slave->dev->name);
 			}
+
+			write_unlock_bh(&bond->curr_slave_lock);
+			read_unlock(&bond->lock);
+
+			netdev_bonding_change(bond->dev);
+
+			read_lock(&bond->lock);
+			write_lock_bh(&bond->curr_slave_lock);
 		}
 	}
 }
-- 
1.7.6.5


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 3/4] review For Oracle bug 14470382

[jianhai luan]

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]


On 2013-1-18 9:46, jianhai luan wrote:
> orabug: 14470382
>
> When Dom0's network environment change, for example the issue, DomU 
> should send send gratuitous ARP initially to notify how to reach it.
>
> To fix the bug, we should modify backend and frontend at the same 
> time. For fixing frontend, i filer the new bug 16182568. The review 
> only to backend.
>
> For fixing the backend, I have 4 patch for review.
>
[PATCH]Change function netdev_bonding_change
> Thanks,
> Jason


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-bonding-remap-muticast-addresses-without-using-dev_c.patch --]
[-- Type: text/plain; charset=gb18030; name="0003-bonding-remap-muticast-addresses-without-using-dev_c.patch", Size: 3050 bytes --]

From 75c78500ddad74b229cd0691496b8549490496a2 Mon Sep 17 00:00:00 2001
From: Moni Shoua <monis@voltaire.com>
Date: Tue, 15 Sep 2009 02:37:40 -0700
Subject: [PATCH] bonding: remap muticast addresses without using dev_close()
 and dev_open()

This patch fixes commit e36b9d16c6a6d0f59803b3ef04ff3c22c3844c10. The approach
there is to call dev_close()/dev_open() whenever the device type is changed in
order to remap the device IP multicast addresses to HW multicast addresses.
This approach suffers from 2 drawbacks:

*. It assumes tha the device is UP when calling dev_close(), or otherwise
   dev_close() has no affect. It is worth to mention that initscripts (Redhat)
   and sysconfig (Suse) doesn't act the same in this matter.
*. dev_close() has other side affects, like deleting entries from the routing
   table, which might be unnecessary.

The fix here is to directly remap the IP multicast addresses to HW multicast
addresses for a bonding device that changes its type, and nothing else.

Reported-by:   Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Moni Shoua <monis@voltaire.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff -Nur linux-2.6.18.i686.0002/drivers/net/bonding/bond_main.c linux-2.6.18.i686/drivers/net/bonding/bond_main.c
--- linux-2.6.18.i686.0002/drivers/net/bonding/bond_main.c	2013-01-17 13:54:39.000000000 +0800
+++ linux-2.6.18.i686/drivers/net/bonding/bond_main.c	2013-01-17 14:07:49.000000000 +0800
@@ -1103,7 +1103,7 @@
 			write_unlock_bh(&bond->curr_slave_lock);
 			read_unlock(&bond->lock);
 
-			netdev_bonding_change(bond->dev);
+			netdev_bonding_change(bond->dev, NETDEV_BONDING_FAILOVER);
 
 			read_lock(&bond->lock);
 			write_lock_bh(&bond->curr_slave_lock);
diff -Nur linux-2.6.18.i686.0002/include/linux/netdevice.h linux-2.6.18.i686/include/linux/netdevice.h
--- linux-2.6.18.i686.0002/include/linux/netdevice.h	2013-01-17 13:53:54.000000000 +0800
+++ linux-2.6.18.i686/include/linux/netdevice.h	2013-01-17 14:09:02.000000000 +0800
@@ -1004,7 +1004,8 @@
 extern void		dev_set_promiscuity(struct net_device *dev, int inc);
 extern void		dev_set_allmulti(struct net_device *dev, int inc);
 extern void		netdev_state_change(struct net_device *dev);
-extern void		netdev_bonding_change(struct net_device *dev);
+extern void		netdev_bonding_change(struct net_device *dev,
+					      unsigned long event);
 extern void		netdev_features_change(struct net_device *dev);
 /* Load a device via the kmod */
 extern void		dev_load(const char *name);
diff -Nur linux-2.6.18.i686.0002/net/core/dev.c linux-2.6.18.i686/net/core/dev.c
--- linux-2.6.18.i686.0002/net/core/dev.c	2013-01-17 13:53:54.000000000 +0800
+++ linux-2.6.18.i686/net/core/dev.c	2013-01-17 14:11:29.000000000 +0800
@@ -797,9 +797,9 @@
 	}
 }
 
-void netdev_bonding_change(struct net_device *dev)
+void netdev_bonding_change(struct net_device *dev, unsigned long event)
 {
-	call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, dev);
+	call_netdevice_notifiers(event, dev);
 }
 EXPORT_SYMBOL(netdev_bonding_change);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 4/4] review For Oracle bug 14470382

[jianhai luan]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]


On 2013-1-18 9:46, jianhai luan wrote:
> orabug: 14470382
>
> When Dom0's network environment change, for example the issue, DomU 
> should send send gratuitous ARP initially to notify how to reach it.
>
> To fix the bug, we should modify backend and frontend at the same 
> time. For fixing frontend, i filer the new bug 16182568. The review 
> only to backend.
>
> For fixing the backend, I have 4 patch for review.
[PATCH] Notify DomU to send gratuitous ARP initially by 
Connected->Connected transition
>
> Thanks,
> Jason


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-xen-netback-notify-frontend-to-send-gratuitous-ARP.patch --]
[-- Type: text/plain; charset=gb18030; name="0004-xen-netback-notify-frontend-to-send-gratuitous-ARP.patch", Size: 3567 bytes --]

From 6d27fe89373b000d750ae0a8b9fb831357e286ed Mon Sep 17 00:00:00 2001
From: Jason Luan <jianhai.luan@oracle.com> 
Date: Fri, 28 Dec 2012 15:43:06 +0800 
Subject: [PATCH] xen-netback notify frontend to send gratuitous ARP.

In the real network environment, some cause will lead
to Active-Backup mode bonding chose new actived port.
After that, the trffic, destinated to DomU by inactived
port (former actived port), will be unreachable at now.
DomU should send gratutious ARP initialtivly to find the
new corrected path.

By netback's Connected->Connected transition, frontend 
will watch the change, and send gratuitous ARP.

Signed-off-by: Jason Luan <jianhai.luan@oracle.com>
---

diff -Nur linux-2.6.18.i686.orig/drivers/xen/netback/common.h linux-2.6.18.i686/drivers/xen/netback/common.h
--- linux-2.6.18.i686.orig/drivers/xen/netback/common.h	2013-01-17 14:21:50.000000000 +0800
+++ linux-2.6.18.i686/drivers/xen/netback/common.h	2013-01-17 14:32:48.000000000 +0800
@@ -151,6 +151,8 @@
 	void *netback_accel_priv;
 	/* The accelerator that this backend is currently using */
 	struct netback_accelerator *accelerator;
+
+	struct notifier_block vif_notifier;
 };
 
 #define NETBACK_ACCEL_VERSION 0x00010001
diff -Nur linux-2.6.18.i686.orig/drivers/xen/netback/xenbus.c linux-2.6.18.i686/drivers/xen/netback/xenbus.c
--- linux-2.6.18.i686.orig/drivers/xen/netback/xenbus.c	2013-01-17 14:21:50.000000000 +0800
+++ linux-2.6.18.i686/drivers/xen/netback/xenbus.c	2013-01-17 14:34:02.000000000 +0800
@@ -33,10 +33,73 @@
 static void connect(struct backend_info *);
 static void backend_create_netif(struct backend_info *be);
 
+/**
+ * By Connected->Connected transition, netfront will watch the change and
+ * send gratuitous ARP.
+ */
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	struct xenbus_transaction xbt;
+	int err;
+
+	if (xenbus_read_driver_state(dev->nodename) != XenbusStateConnected)
+		return;
+
+again:
+	err = xenbus_transaction_start(&xbt);
+	if (err) {
+		printk(KERN_ALERT "Error starting transaction");
+		return;
+	}
+
+	err = xenbus_printf(xbt, dev->nodename, "state", "%d", dev->state);
+	if(err) {
+		printk(KERN_ALERT "Error writing the state");
+		xenbus_transaction_end(xbt, 1);
+		return;
+	}
+	
+	err = xenbus_transaction_end(xbt, 0);
+	if (err == -EAGAIN)
+		goto again;
+	if (err)
+		printk(KERN_ALERT "Error ending transaction");
+
+	return;
+}
+
+#define nb_to_backend(nb) container_of(nb, struct backend_info, vif_notifier)
+/**
+ * When network condition of vif change, notify the frontend.
+ */
+static int netback_netdev_event(struct notifier_block *this,
+		unsigned long event, void *ptr)
+{
+	struct net_device *event_dev = ptr;
+	struct backend_info *be = nb_to_backend(this);
+
+	pr_debug("event_dev: %s, event: %lx\n",
+		event_dev ? event_dev->name : "None", event);
+
+	if (!be->netif)
+		return NOTIFY_DONE;
+	
+	switch (event) {
+	case NETDEV_BONDING_FAILOVER:
+		/* Notify frontend to Send gratuitous ARP */
+		notify_front_arping(be->dev);
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 static int netback_remove(struct xenbus_device *dev)
 {
 	struct backend_info *be = dev->dev.driver_data;
 
+	unregister_netdevice_notifier(&be->vif_notifier);
+
 	netback_remove_accelerators(be, dev);
 
 	if (be->netif) {
@@ -132,6 +195,10 @@
 	/* This kicks hotplug scripts, so do it immediately. */
 	backend_create_netif(be);
 
+	/* Register Frontend Event Notify */
+	be->vif_notifier.notifier_call = netback_netdev_event;
+	register_netdevice_notifier(&be->vif_notifier);
+
 	return 0;
 
 abort_transaction:

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [Test Result] review For Oracle bug 14470382

[jianhai luan]

On 2013-1-18 9:46, jianhai luan wrote:
> orabug: 14470382
>
> When Dom0's network environment change, for example the issue, DomU 
> should send send gratuitous ARP initially to notify how to reach it.
>
> To fix the bug, we should modify backend and frontend at the same 
> time. For fixing frontend, i filer the new bug 16182568. The review 
> only to backend.
>
> For fixing the backend, I have 4 patch for review.
>
The Above PATCH have been tested at some systems (Linux, Window, 
Solaris, NetBsd). The result is below:

Linux     PASS(send ARP, no other issue)
Window    PASS(don't send ARP, but no other issue)
Solaris   PASS(don't send ARP, but no other issue)
NetBSD    PASS(don't send ARP, but no other issue)

> Thanks,
> Jason

Re: [Test Result] review For Oracle bug 14470382

[jianhai luan]

Sorry, i add wrong mail address. Because the patches for review in 
internal oracle only, please ignore it.

On 2013-1-18 10:03, jianhai luan wrote:
>
> On 2013-1-18 9:46, jianhai luan wrote:
>> orabug: 14470382
>>
>> When Dom0's network environment change, for example the issue, DomU 
>> should send send gratuitous ARP initially to notify how to reach it.
>>
>> To fix the bug, we should modify backend and frontend at the same 
>> time. For fixing frontend, i filer the new bug 16182568. The review 
>> only to backend.
>>
>> For fixing the backend, I have 4 patch for review.
>>
> The Above PATCH have been tested at some systems (Linux, Window, 
> Solaris, NetBsd). The result is below:
>
> Linux     PASS(send ARP, no other issue)
> Window    PASS(don't send ARP, but no other issue)
> Solaris   PASS(don't send ARP, but no other issue)
> NetBSD    PASS(don't send ARP, but no other issue)
>
>> Thanks,
>> Jason
>

Re: [Test Result] review For Oracle bug 14470382

[jianhai luan]

Because i add wrong mail address(xen-devel@lists.xensorces.com), please 
delete the address when you replay.

I'm so sorry for your inconvenient.

Thanks,
Jason

On 2013-1-18 10:03, jianhai luan wrote:
>
> On 2013-1-18 9:46, jianhai luan wrote:
>> orabug: 14470382
>>
>> When Dom0's network environment change, for example the issue, DomU 
>> should send send gratuitous ARP initially to notify how to reach it.
>>
>> To fix the bug, we should modify backend and frontend at the same 
>> time. For fixing frontend, i filer the new bug 16182568. The review 
>> only to backend.
>>
>> For fixing the backend, I have 4 patch for review.
>>
> The Above PATCH have been tested at some systems (Linux, Window, 
> Solaris, NetBsd). The result is below:
>
> Linux     PASS(send ARP, no other issue)
> Window    PASS(don't send ARP, but no other issue)
> Solaris   PASS(don't send ARP, but no other issue)
> NetBSD    PASS(don't send ARP, but no other issue)
>
>> Thanks,
>> Jason
>

Re: [PATCH 4/4] review For Oracle bug 14470382

[Dan Carpenter]

On Fri, Jan 18, 2013 at 09:57:48AM +0800, jianhai luan wrote:
> +/**
> + * By Connected->Connected transition, netfront will watch the change and
> + * send gratuitous ARP.
> + */
> +static void notify_front_arping(struct xenbus_device *dev)
> +{
> +	struct xenbus_transaction xbt;
> +	int err;
> +
> +	if (xenbus_read_driver_state(dev->nodename) != XenbusStateConnected)
> +		return;
> +
> +again:
> +	err = xenbus_transaction_start(&xbt);
> +	if (err) {
> +		printk(KERN_ALERT "Error starting transaction");
> +		return;
> +	}
> +
> +	err = xenbus_printf(xbt, dev->nodename, "state", "%d", dev->state);
> +	if(err) {
> +		printk(KERN_ALERT "Error writing the state");
> +		xenbus_transaction_end(xbt, 1);
> +		return;
> +	}
> +	
> +	err = xenbus_transaction_end(xbt, 0);
> +	if (err == -EAGAIN)
> +		goto again;

Why would we get a -EAGAIN here?  My reading says that it would have
to come as the msg->body from the XenServer.  I would be tempted to
just ignore those errors.

(I don't know anything about Xen though, so I'm probably full of
crap).

regards,
dan carpenter

Re: [PATCH 4/4] review For Oracle bug 14470382

[Ian Campbell]

On Fri, 2013-01-18 at 06:45 +0000, Dan Carpenter wrote:
> On Fri, Jan 18, 2013 at 09:57:48AM +0800, jianhai luan wrote:
> > +/**
> > + * By Connected->Connected transition, netfront will watch the change and
> > + * send gratuitous ARP.
> > + */
> > +static void notify_front_arping(struct xenbus_device *dev)
> > +{
> > +	struct xenbus_transaction xbt;
> > +	int err;
> > +
> > +	if (xenbus_read_driver_state(dev->nodename) != XenbusStateConnected)
> > +		return;
> > +
> > +again:
> > +	err = xenbus_transaction_start(&xbt);
> > +	if (err) {
> > +		printk(KERN_ALERT "Error starting transaction");
> > +		return;
> > +	}
> > +
> > +	err = xenbus_printf(xbt, dev->nodename, "state", "%d", dev->state);
> > +	if(err) {
> > +		printk(KERN_ALERT "Error writing the state");
> > +		xenbus_transaction_end(xbt, 1);
> > +		return;
> > +	}
> > +	
> > +	err = xenbus_transaction_end(xbt, 0);
> > +	if (err == -EAGAIN)
> > +		goto again;
> 
> Why would we get a -EAGAIN here?  My reading says that it would have
> to come as the msg->body from the XenServer.  I would be tempted to
> just ignore those errors.

EAGAIN is what the xenstore daemon returns when a transaction fails,
e.g. due to a conflicting change. 

However a single xenbus_printf as used here doesn't need a transaction
since it is only a single atomic operation.

On the other hand I'm not convinced that the initial read of the current
state didn't ought to be inside the transaction too, although I've not
thought about it terribly hard...

Ian.

Re: [PATCH 4/4] review For Oracle bug 14470382

[Jan Beulich]

>>> On 18.01.13 at 11:08, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> However a single xenbus_printf as used here doesn't need a transaction
> since it is only a single atomic operation.

Right.

> On the other hand I'm not convinced that the initial read of the current
> state didn't ought to be inside the transaction too, although I've not
> thought about it terribly hard...

I don't think so - this is just a safeguard, and I think it could as
well look directly at dev->state.

Jan

Re: [PATCH 4/4] review For Oracle bug 14470382

[Jason Luan]

于 2013年01月18日 18:26, Jan Beulich 写道:
>>>> On 18.01.13 at 11:08, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> However a single xenbus_printf as used here doesn't need a transaction
>> since it is only a single atomic operation.
> Right.

If the fact is that, the code will be simple. i will modify it.
>
>> On the other hand I'm not convinced that the initial read of the current
>> state didn't ought to be inside the transaction too, although I've not
>> thought about it terribly hard...
> I don't think so - this is just a safeguard, and I think it could as
> well look directly at dev->state.

Right, this is just a safeguard. you think that the below way is better?

if (dev->state != XenbusStateConnected)
     return;

>
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Thanks,
Jason


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