Message ID | 1363056171-5854-2-git-send-email-asias@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Mar 12, 2013 at 10:42:48AM +0800, Asias He wrote: > tv_tpg->tv_tpg_vhost_count should be protected by tv_tpg->tv_tpg_mutex. > > Signed-off-by: Asias He <asias@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > drivers/vhost/tcm_vhost.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c > index 9951297..b3e50d7 100644 > --- a/drivers/vhost/tcm_vhost.c > +++ b/drivers/vhost/tcm_vhost.c > @@ -860,9 +860,11 @@ static int vhost_scsi_clear_endpoint( > if (!tv_tpg) > continue; > > + mutex_lock(&tv_tpg->tv_tpg_mutex); > tv_tport = tv_tpg->tport; > if (!tv_tport) { > ret = -ENODEV; > + mutex_unlock(&tv_tpg->tv_tpg_mutex); > goto err; > } > > @@ -872,11 +874,13 @@ static int vhost_scsi_clear_endpoint( > tv_tport->tport_name, tv_tpg->tport_tpgt, > t->vhost_wwpn, t->vhost_tpgt); > ret = -EINVAL; > + mutex_unlock(&tv_tpg->tv_tpg_mutex); > goto err; > } > tv_tpg->tv_tpg_vhost_count--; > vs->vs_tpg[target] = NULL; > vs->vs_endpoint = false; > + mutex_unlock(&tv_tpg->tv_tpg_mutex); > } > mutex_unlock(&vs->dev.mutex); > return 0; Does the error handling have to be so messy? How about another label which does unlock and goto there? > -- > 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 12, 2013 at 01:13:44PM +0200, Michael S. Tsirkin wrote: > On Tue, Mar 12, 2013 at 10:42:48AM +0800, Asias He wrote: > > tv_tpg->tv_tpg_vhost_count should be protected by tv_tpg->tv_tpg_mutex. > > > > Signed-off-by: Asias He <asias@redhat.com> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > drivers/vhost/tcm_vhost.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c > > index 9951297..b3e50d7 100644 > > --- a/drivers/vhost/tcm_vhost.c > > +++ b/drivers/vhost/tcm_vhost.c > > @@ -860,9 +860,11 @@ static int vhost_scsi_clear_endpoint( > > if (!tv_tpg) > > continue; > > > > + mutex_lock(&tv_tpg->tv_tpg_mutex); > > tv_tport = tv_tpg->tport; > > if (!tv_tport) { > > ret = -ENODEV; > > + mutex_unlock(&tv_tpg->tv_tpg_mutex); > > goto err; > > } > > > > @@ -872,11 +874,13 @@ static int vhost_scsi_clear_endpoint( > > tv_tport->tport_name, tv_tpg->tport_tpgt, > > t->vhost_wwpn, t->vhost_tpgt); > > ret = -EINVAL; > > + mutex_unlock(&tv_tpg->tv_tpg_mutex); > > goto err; > > } > > tv_tpg->tv_tpg_vhost_count--; > > vs->vs_tpg[target] = NULL; > > vs->vs_endpoint = false; > > + mutex_unlock(&tv_tpg->tv_tpg_mutex); > > } > > mutex_unlock(&vs->dev.mutex); > > return 0; > > Does the error handling have to be so messy? > How about another label which does unlock and goto there? Well, I will do it. > > -- > > 1.8.1.4
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c index 9951297..b3e50d7 100644 --- a/drivers/vhost/tcm_vhost.c +++ b/drivers/vhost/tcm_vhost.c @@ -860,9 +860,11 @@ static int vhost_scsi_clear_endpoint( if (!tv_tpg) continue; + mutex_lock(&tv_tpg->tv_tpg_mutex); tv_tport = tv_tpg->tport; if (!tv_tport) { ret = -ENODEV; + mutex_unlock(&tv_tpg->tv_tpg_mutex); goto err; } @@ -872,11 +874,13 @@ static int vhost_scsi_clear_endpoint( tv_tport->tport_name, tv_tpg->tport_tpgt, t->vhost_wwpn, t->vhost_tpgt); ret = -EINVAL; + mutex_unlock(&tv_tpg->tv_tpg_mutex); goto err; } tv_tpg->tv_tpg_vhost_count--; vs->vs_tpg[target] = NULL; vs->vs_endpoint = false; + mutex_unlock(&tv_tpg->tv_tpg_mutex); } mutex_unlock(&vs->dev.mutex); return 0;