diff mbox series

bus/vdev: fix device argument corrupt after bus scan

Message ID 20181025033036.23680-1-qi.z.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series bus/vdev: fix device argument corrupt after bus scan |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Qi Zhang Oct. 25, 2018, 3:30 a.m. UTC 
  It's not necessary to insert device argment to devargs_list
during bus scan, but this happens when we try to attach a
device on secondary process. The patch fix the issue.

Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
Cc: stable@dpdk.org

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/bus/vdev/vdev.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Gaëtan Rivet Oct. 25, 2018, 9:51 a.m. UTC | #1 
On Thu, Oct 25, 2018 at 11:30:36AM +0800, Qi Zhang wrote:
> It's not necessary to insert device argment to devargs_list
> during bus scan, but this happens when we try to attach a
> device on secondary process. The patch fix the issue.
> 
> Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
>  drivers/bus/vdev/vdev.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index 688e31c21..818a2bfc2 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -202,7 +202,9 @@ alloc_devargs(const char *name, const char *args)
>  }
>  
>  static int
> -insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
> +insert_vdev(const char *name, const char *args,
> +		struct rte_vdev_device **p_dev,
> +		bool init)

Why is vdev the only bus not using hotplug API itself and reimplementing
it on its own?

It should not have to insert devargs at all, not even in the primary
process. If it called rte_dev_probe(), this would normally already be
properly handled I think.

>  {
>  	struct rte_vdev_device *dev;
>  	struct rte_devargs *devargs;
> @@ -237,7 +239,8 @@ insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
>  	}
>  
>  	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
> -	rte_devargs_insert(devargs);
> +	if (init)
> +		rte_devargs_insert(devargs);
>  
>  	if (p_dev)
>  		*p_dev = dev;
> @@ -257,7 +260,7 @@ rte_vdev_init(const char *name, const char *args)
>  	int ret;
>  
>  	rte_spinlock_recursive_lock(&vdev_device_list_lock);
> -	ret = insert_vdev(name, args, &dev);
> +	ret = insert_vdev(name, args, &dev, true);
>  	if (ret == 0) {
>  		ret = vdev_probe_all_drivers(dev);
>  		if (ret) {
> @@ -383,7 +386,7 @@ vdev_action(const struct rte_mp_msg *mp_msg, const void *peer)
>  		break;
>  	case VDEV_SCAN_ONE:
>  		VDEV_LOG(INFO, "receive vdev, %s", in->name);
> -		ret = insert_vdev(in->name, NULL, NULL);
> +		ret = insert_vdev(in->name, NULL, NULL, false);
>  		if (ret == -EEXIST)
>  			VDEV_LOG(DEBUG, "device already exist, %s", in->name);
>  		else if (ret < 0)
> -- 
> 2.13.6
>
Qi Zhang Oct. 25, 2018, 2:56 p.m. UTC | #2 
> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Thursday, October 25, 2018 4:51 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: thomas@monjalon.net; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH] bus/vdev: fix device argument corrupt after bus scan
> 
> On Thu, Oct 25, 2018 at 11:30:36AM +0800, Qi Zhang wrote:
> > It's not necessary to insert device argment to devargs_list during bus
> > scan, but this happens when we try to attach a device on secondary
> > process. The patch fix the issue.
> >
> > Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> >  drivers/bus/vdev/vdev.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c index
> > 688e31c21..818a2bfc2 100644
> > --- a/drivers/bus/vdev/vdev.c
> > +++ b/drivers/bus/vdev/vdev.c
> > @@ -202,7 +202,9 @@ alloc_devargs(const char *name, const char *args)
> > }
> >
> >  static int
> > -insert_vdev(const char *name, const char *args, struct
> > rte_vdev_device **p_dev)
> > +insert_vdev(const char *name, const char *args,
> > +		struct rte_vdev_device **p_dev,
> > +		bool init)
> 
> Why is vdev the only bus not using hotplug API itself and reimplementing it
> on its own?

I don't know the history, 
but replace insert_vdev with hotplug API will not solve all the problem (see my below comments)

> 
> It should not have to insert devargs at all, not even in the primary process. If
> it called rte_dev_probe(), this would normally already be properly handled I
> think.

Currently insert_vdev is shared by two scenarios
1. rte_vdev_init, which is called by application to attach a new device, I agree it's better to have rte_dev_probe here so, no need to have insert_vdev here.
2. during secondary's vdev->scan when it receive a SCAN_ONE event from primary, it should not call rte_devargs_insert, 
The patch is going to fix the issue on secondary scenario and we can do the cleanup for first issue in a separate patch



> 
> >  {
> >  	struct rte_vdev_device *dev;
> >  	struct rte_devargs *devargs;
> > @@ -237,7 +239,8 @@ insert_vdev(const char *name, const char *args,
> struct rte_vdev_device **p_dev)
> >  	}
> >
> >  	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
> > -	rte_devargs_insert(devargs);
> > +	if (init)
> > +		rte_devargs_insert(devargs);
> >
> >  	if (p_dev)
> >  		*p_dev = dev;
> > @@ -257,7 +260,7 @@ rte_vdev_init(const char *name, const char *args)
> >  	int ret;
> >
> >  	rte_spinlock_recursive_lock(&vdev_device_list_lock);
> > -	ret = insert_vdev(name, args, &dev);
> > +	ret = insert_vdev(name, args, &dev, true);
> >  	if (ret == 0) {
> >  		ret = vdev_probe_all_drivers(dev);
> >  		if (ret) {
> > @@ -383,7 +386,7 @@ vdev_action(const struct rte_mp_msg *mp_msg,
> const void *peer)
> >  		break;
> >  	case VDEV_SCAN_ONE:
> >  		VDEV_LOG(INFO, "receive vdev, %s", in->name);
> > -		ret = insert_vdev(in->name, NULL, NULL);
> > +		ret = insert_vdev(in->name, NULL, NULL, false);
> >  		if (ret == -EEXIST)
> >  			VDEV_LOG(DEBUG, "device already exist, %s", in->name);
> >  		else if (ret < 0)
> > --
> > 2.13.6
> >
> 
> --
> Gaëtan Rivet
> 6WIND
Gaëtan Rivet Oct. 25, 2018, 3:03 p.m. UTC | #3 
On Thu, Oct 25, 2018 at 02:56:55PM +0000, Zhang, Qi Z wrote:
> 
> 
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > Sent: Thursday, October 25, 2018 4:51 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: thomas@monjalon.net; dev@dpdk.org; stable@dpdk.org
> > Subject: Re: [PATCH] bus/vdev: fix device argument corrupt after bus scan
> > 
> > On Thu, Oct 25, 2018 at 11:30:36AM +0800, Qi Zhang wrote:
> > > It's not necessary to insert device argment to devargs_list during bus
> > > scan, but this happens when we try to attach a device on secondary
> > > process. The patch fix the issue.
> > >
> > > Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > > ---
> > >  drivers/bus/vdev/vdev.c | 11 +++++++----
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c index
> > > 688e31c21..818a2bfc2 100644
> > > --- a/drivers/bus/vdev/vdev.c
> > > +++ b/drivers/bus/vdev/vdev.c
> > > @@ -202,7 +202,9 @@ alloc_devargs(const char *name, const char *args)
> > > }
> > >
> > >  static int
> > > -insert_vdev(const char *name, const char *args, struct
> > > rte_vdev_device **p_dev)
> > > +insert_vdev(const char *name, const char *args,
> > > +		struct rte_vdev_device **p_dev,
> > > +		bool init)
> > 
> > Why is vdev the only bus not using hotplug API itself and reimplementing it
> > on its own?
> 
> I don't know the history, 
> but replace insert_vdev with hotplug API will not solve all the problem (see my below comments)
> 
> > 
> > It should not have to insert devargs at all, not even in the primary process. If
> > it called rte_dev_probe(), this would normally already be properly handled I
> > think.
> 
> Currently insert_vdev is shared by two scenarios
> 1. rte_vdev_init, which is called by application to attach a new device, I agree it's better to have rte_dev_probe here so, no need to have insert_vdev here.
> 2. during secondary's vdev->scan when it receive a SCAN_ONE event from primary, it should not call rte_devargs_insert, 
> The patch is going to fix the issue on secondary scenario and we can do the cleanup for first issue in a separate patch

In 2., won't dev_probe() check for secondary process context and in this
case, send the request to primary, which will see that the device is
already probed, which would thus fix your issue?

My take on it is that it seems to be fixing both your issue and cleaning
up history.

> 
> 
> 
> > 
> > >  {
> > >  	struct rte_vdev_device *dev;
> > >  	struct rte_devargs *devargs;
> > > @@ -237,7 +239,8 @@ insert_vdev(const char *name, const char *args,
> > struct rte_vdev_device **p_dev)
> > >  	}
> > >
> > >  	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
> > > -	rte_devargs_insert(devargs);
> > > +	if (init)
> > > +		rte_devargs_insert(devargs);
> > >
> > >  	if (p_dev)
> > >  		*p_dev = dev;
> > > @@ -257,7 +260,7 @@ rte_vdev_init(const char *name, const char *args)
> > >  	int ret;
> > >
> > >  	rte_spinlock_recursive_lock(&vdev_device_list_lock);
> > > -	ret = insert_vdev(name, args, &dev);
> > > +	ret = insert_vdev(name, args, &dev, true);
> > >  	if (ret == 0) {
> > >  		ret = vdev_probe_all_drivers(dev);
> > >  		if (ret) {
> > > @@ -383,7 +386,7 @@ vdev_action(const struct rte_mp_msg *mp_msg,
> > const void *peer)
> > >  		break;
> > >  	case VDEV_SCAN_ONE:
> > >  		VDEV_LOG(INFO, "receive vdev, %s", in->name);
> > > -		ret = insert_vdev(in->name, NULL, NULL);
> > > +		ret = insert_vdev(in->name, NULL, NULL, false);
> > >  		if (ret == -EEXIST)
> > >  			VDEV_LOG(DEBUG, "device already exist, %s", in->name);
> > >  		else if (ret < 0)
> > > --
> > > 2.13.6
> > >
> > 
> > --
> > Gaëtan Rivet
> > 6WIND
Qi Zhang Oct. 25, 2018, 3:18 p.m. UTC | #4 
> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Thursday, October 25, 2018 10:03 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: thomas@monjalon.net; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH] bus/vdev: fix device argument corrupt after bus scan
> 
> On Thu, Oct 25, 2018 at 02:56:55PM +0000, Zhang, Qi Z wrote:
> >
> >
> > > -----Original Message-----
> > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > > Sent: Thursday, October 25, 2018 4:51 AM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > Cc: thomas@monjalon.net; dev@dpdk.org; stable@dpdk.org
> > > Subject: Re: [PATCH] bus/vdev: fix device argument corrupt after bus
> > > scan
> > >
> > > On Thu, Oct 25, 2018 at 11:30:36AM +0800, Qi Zhang wrote:
> > > > It's not necessary to insert device argment to devargs_list during
> > > > bus scan, but this happens when we try to attach a device on
> > > > secondary process. The patch fix the issue.
> > > >
> > > > Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > ---
> > > >  drivers/bus/vdev/vdev.c | 11 +++++++----
> > > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> > > > index
> > > > 688e31c21..818a2bfc2 100644
> > > > --- a/drivers/bus/vdev/vdev.c
> > > > +++ b/drivers/bus/vdev/vdev.c
> > > > @@ -202,7 +202,9 @@ alloc_devargs(const char *name, const char
> > > > *args) }
> > > >
> > > >  static int
> > > > -insert_vdev(const char *name, const char *args, struct
> > > > rte_vdev_device **p_dev)
> > > > +insert_vdev(const char *name, const char *args,
> > > > +		struct rte_vdev_device **p_dev,
> > > > +		bool init)
> > >
> > > Why is vdev the only bus not using hotplug API itself and
> > > reimplementing it on its own?
> >
> > I don't know the history,
> > but replace insert_vdev with hotplug API will not solve all the
> > problem (see my below comments)
> >
> > >
> > > It should not have to insert devargs at all, not even in the primary
> > > process. If it called rte_dev_probe(), this would normally already
> > > be properly handled I think.
> >
> > Currently insert_vdev is shared by two scenarios 1. rte_vdev_init,
> > which is called by application to attach a new device, I agree it's better to
> have rte_dev_probe here so, no need to have insert_vdev here.
> > 2. during secondary's vdev->scan when it receive a SCAN_ONE event from
> > primary, it should not call rte_devargs_insert, The patch is going to
> > fix the issue on secondary scenario and we can do the cleanup for
> > first issue in a separate patch
> 
> In 2., won't dev_probe() check for secondary process context and in this case,
> send the request to primary, which will see that the device is already probed,
> which would thus fix your issue?

No, It's not the case that we issue a device attaching from secondary, but the case that we try to sync with primary for a device already be attached 
So we are in the context of dev->scan, we should not do dev_probe in dev->scan, since dev_probe will call dev->scan again, then it go dead loop.
> 
> My take on it is that it seems to be fixing both your issue and cleaning up
> history.
> 
> >
> >
> >
> > >
> > > >  {
> > > >  	struct rte_vdev_device *dev;
> > > >  	struct rte_devargs *devargs;
> > > > @@ -237,7 +239,8 @@ insert_vdev(const char *name, const char
> > > > *args,
> > > struct rte_vdev_device **p_dev)
> > > >  	}
> > > >
> > > >  	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
> > > > -	rte_devargs_insert(devargs);
> > > > +	if (init)
> > > > +		rte_devargs_insert(devargs);
> > > >
> > > >  	if (p_dev)
> > > >  		*p_dev = dev;
> > > > @@ -257,7 +260,7 @@ rte_vdev_init(const char *name, const char
> *args)
> > > >  	int ret;
> > > >
> > > >  	rte_spinlock_recursive_lock(&vdev_device_list_lock);
> > > > -	ret = insert_vdev(name, args, &dev);
> > > > +	ret = insert_vdev(name, args, &dev, true);
> > > >  	if (ret == 0) {
> > > >  		ret = vdev_probe_all_drivers(dev);
> > > >  		if (ret) {
> > > > @@ -383,7 +386,7 @@ vdev_action(const struct rte_mp_msg
> *mp_msg,
> > > const void *peer)
> > > >  		break;
> > > >  	case VDEV_SCAN_ONE:
> > > >  		VDEV_LOG(INFO, "receive vdev, %s", in->name);
> > > > -		ret = insert_vdev(in->name, NULL, NULL);
> > > > +		ret = insert_vdev(in->name, NULL, NULL, false);
> > > >  		if (ret == -EEXIST)
> > > >  			VDEV_LOG(DEBUG, "device already exist, %s", in->name);
> > > >  		else if (ret < 0)
> > > > --
> > > > 2.13.6
> > > >
> > >
> > > --
> > > Gaëtan Rivet
> > > 6WIND
> 
> --
> Gaëtan Rivet
> 6WIND
Gaëtan Rivet Oct. 25, 2018, 3:26 p.m. UTC | #5 
On Thu, Oct 25, 2018 at 03:18:20PM +0000, Zhang, Qi Z wrote:
> 
> 
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > Sent: Thursday, October 25, 2018 10:03 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: thomas@monjalon.net; dev@dpdk.org; stable@dpdk.org
> > Subject: Re: [PATCH] bus/vdev: fix device argument corrupt after bus scan
> > 
> > On Thu, Oct 25, 2018 at 02:56:55PM +0000, Zhang, Qi Z wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > > > Sent: Thursday, October 25, 2018 4:51 AM
> > > > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > > Cc: thomas@monjalon.net; dev@dpdk.org; stable@dpdk.org
> > > > Subject: Re: [PATCH] bus/vdev: fix device argument corrupt after bus
> > > > scan
> > > >
> > > > On Thu, Oct 25, 2018 at 11:30:36AM +0800, Qi Zhang wrote:
> > > > > It's not necessary to insert device argment to devargs_list during
> > > > > bus scan, but this happens when we try to attach a device on
> > > > > secondary process. The patch fix the issue.
> > > > >
> > > > > Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > ---
> > > > >  drivers/bus/vdev/vdev.c | 11 +++++++----
> > > > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> > > > > index
> > > > > 688e31c21..818a2bfc2 100644
> > > > > --- a/drivers/bus/vdev/vdev.c
> > > > > +++ b/drivers/bus/vdev/vdev.c
> > > > > @@ -202,7 +202,9 @@ alloc_devargs(const char *name, const char
> > > > > *args) }
> > > > >
> > > > >  static int
> > > > > -insert_vdev(const char *name, const char *args, struct
> > > > > rte_vdev_device **p_dev)
> > > > > +insert_vdev(const char *name, const char *args,
> > > > > +		struct rte_vdev_device **p_dev,
> > > > > +		bool init)
> > > >
> > > > Why is vdev the only bus not using hotplug API itself and
> > > > reimplementing it on its own?
> > >
> > > I don't know the history,
> > > but replace insert_vdev with hotplug API will not solve all the
> > > problem (see my below comments)
> > >
> > > >
> > > > It should not have to insert devargs at all, not even in the primary
> > > > process. If it called rte_dev_probe(), this would normally already
> > > > be properly handled I think.
> > >
> > > Currently insert_vdev is shared by two scenarios 1. rte_vdev_init,
> > > which is called by application to attach a new device, I agree it's better to
> > have rte_dev_probe here so, no need to have insert_vdev here.
> > > 2. during secondary's vdev->scan when it receive a SCAN_ONE event from
> > > primary, it should not call rte_devargs_insert, The patch is going to
> > > fix the issue on secondary scenario and we can do the cleanup for
> > > first issue in a separate patch
> > 
> > In 2., won't dev_probe() check for secondary process context and in this case,
> > send the request to primary, which will see that the device is already probed,
> > which would thus fix your issue?
> 
> No, It's not the case that we issue a device attaching from secondary, but the case that we try to sync with primary for a device already be attached 
> So we are in the context of dev->scan, we should not do dev_probe in dev->scan, since dev_probe will call dev->scan again, then it go dead loop.
> > 
> > My take on it is that it seems to be fixing both your issue and cleaning up
> > history.
> > 

Ok!
As long as you are confident this is the simplest way to write this, while
taking into consideration the future rework of vdev_init(), all good :)
Thomas Monjalon Oct. 28, 2018, 5:32 p.m. UTC | #6 
25/10/2018 05:30, Qi Zhang:
> It's not necessary to insert device argment to devargs_list
> during bus scan, but this happens when we try to attach a
> device on secondary process. The patch fix the issue.
> 
> Fixes: cdb068f031c6 ("bus/vdev: scan by multi-process channel")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 688e31c21..818a2bfc2 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -202,7 +202,9 @@  alloc_devargs(const char *name, const char *args)
 }
 
 static int
-insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
+insert_vdev(const char *name, const char *args,
+		struct rte_vdev_device **p_dev,
+		bool init)
 {
 	struct rte_vdev_device *dev;
 	struct rte_devargs *devargs;
@@ -237,7 +239,8 @@  insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
 	}
 
 	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
-	rte_devargs_insert(devargs);
+	if (init)
+		rte_devargs_insert(devargs);
 
 	if (p_dev)
 		*p_dev = dev;
@@ -257,7 +260,7 @@  rte_vdev_init(const char *name, const char *args)
 	int ret;
 
 	rte_spinlock_recursive_lock(&vdev_device_list_lock);
-	ret = insert_vdev(name, args, &dev);
+	ret = insert_vdev(name, args, &dev, true);
 	if (ret == 0) {
 		ret = vdev_probe_all_drivers(dev);
 		if (ret) {
@@ -383,7 +386,7 @@  vdev_action(const struct rte_mp_msg *mp_msg, const void *peer)
 		break;
 	case VDEV_SCAN_ONE:
 		VDEV_LOG(INFO, "receive vdev, %s", in->name);
-		ret = insert_vdev(in->name, NULL, NULL);
+		ret = insert_vdev(in->name, NULL, NULL, false);
 		if (ret == -EEXIST)
 			VDEV_LOG(DEBUG, "device already exist, %s", in->name);
 		else if (ret < 0)