| Message ID | 1359706570-31808-1-git-send-email-asias@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Asias, On Fri, 2013-02-01 at 16:16 +0800, Asias He wrote: > In order to take advantages of Paolo's multi-queue virito-scsi, we need > multi-target support in tcm_vhost first. Otherwise all the requests go > to one queue and other queues are idle. > > This patch makes: > > 1. All the targets under the wwpn is seen and can be used by guest. > 2. No need to pass the tpgt number in struct vhost_scsi_target to > tcm_vhost.ko. Only wwpn is needed. > 3. We can always pass max_target = 255 to guest now, since we abort the > request who's target id does not exist. > > Changes in v2: > - Handle non-contiguous tpgt > > Signed-off-by: Asias He <asias@redhat.com> > --- So this patch is looks pretty good. Just a few points below.. > drivers/vhost/tcm_vhost.c | 117 ++++++++++++++++++++++++++++++---------------- > drivers/vhost/tcm_vhost.h | 4 +- > 2 files changed, 79 insertions(+), 42 deletions(-) > > diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c > index 218deb6..f1481f0 100644 > --- a/drivers/vhost/tcm_vhost.c > +++ b/drivers/vhost/tcm_vhost.c > @@ -59,8 +59,14 @@ enum { > VHOST_SCSI_VQ_IO = 2, > }; > > +#define VHOST_SCSI_MAX_TARGET 256 > + > struct vhost_scsi { > - struct tcm_vhost_tpg *vs_tpg; /* Protected by vhost_scsi->dev.mutex */ > + /* Protected by vhost_scsi->dev.mutex */ > + struct tcm_vhost_tpg *vs_tpg[VHOST_SCSI_MAX_TARGET]; > + char vs_vhost_wwpn[TRANSPORT_IQN_LEN]; > + bool vs_endpoint; > + > struct vhost_dev dev; > struct vhost_virtqueue vqs[3]; > > @@ -564,13 +570,11 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs) > u32 exp_data_len, data_first, data_num, data_direction; > unsigned out, in, i; > int head, ret; > + u8 target; > > /* Must use ioctl VHOST_SCSI_SET_ENDPOINT */ > - tv_tpg = vs->vs_tpg; > - if (unlikely(!tv_tpg)) { > - pr_err("%s endpoint not set\n", __func__); > + if (unlikely(!vs->vs_endpoint)) > return; > - } > > mutex_lock(&vq->mutex); > vhost_disable_notify(&vs->dev, vq); > @@ -637,6 +641,28 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs) > break; > } > > + /* Extract the tpgt */ > + target = v_req.lun[1]; > + tv_tpg = vs->vs_tpg[target]; > + > + /* Target does not exist, fail the request */ > + if (unlikely(!tv_tpg)) { > + struct virtio_scsi_cmd_resp __user *resp; > + struct virtio_scsi_cmd_resp rsp; > + > + memset(&rsp, 0, sizeof(rsp)); > + rsp.response = VIRTIO_SCSI_S_BAD_TARGET; > + resp = vq->iov[out].iov_base; > + ret = __copy_to_user(resp, &rsp, sizeof(rsp)); > + if (!ret) > + vhost_add_used_and_signal(&vs->dev, > + &vs->vqs[2], head, 0); > + else > + pr_err("Faulted on virtio_scsi_cmd_resp\n"); > + > + continue; > + } > + > exp_data_len = 0; > for (i = 0; i < data_num; i++) > exp_data_len += vq->iov[data_first + i].iov_len; > @@ -745,6 +771,7 @@ static int vhost_scsi_set_endpoint( > { > struct tcm_vhost_tport *tv_tport; > struct tcm_vhost_tpg *tv_tpg; > + bool match = false; > int index; > > mutex_lock(&vs->dev.mutex); > @@ -771,14 +798,11 @@ static int vhost_scsi_set_endpoint( > } > tv_tport = tv_tpg->tport; > > - if (!strcmp(tv_tport->tport_name, t->vhost_wwpn) && > - (tv_tpg->tport_tpgt == t->vhost_tpgt)) { > + if (!strcmp(tv_tport->tport_name, t->vhost_wwpn)) { > tv_tpg->tv_tpg_vhost_count++; > - mutex_unlock(&tv_tpg->tv_tpg_mutex); > - mutex_unlock(&tcm_vhost_mutex); > > mutex_lock(&vs->dev.mutex); > - if (vs->vs_tpg) { > + if (vs->vs_tpg[tv_tpg->tport_tpgt]) { > mutex_unlock(&vs->dev.mutex); > mutex_lock(&tv_tpg->tv_tpg_mutex); > tv_tpg->tv_tpg_vhost_count--; > @@ -786,15 +810,24 @@ static int vhost_scsi_set_endpoint( > return -EEXIST; > } > > - vs->vs_tpg = tv_tpg; > + vs->vs_tpg[tv_tpg->tport_tpgt] = tv_tpg; > smp_mb__after_atomic_inc(); > + match = true; > mutex_unlock(&vs->dev.mutex); > - return 0; > } > mutex_unlock(&tv_tpg->tv_tpg_mutex); > } > mutex_unlock(&tcm_vhost_mutex); > - return -EINVAL; > + > + mutex_lock(&vs->dev.mutex); > + if (match) { > + memcpy(vs->vs_vhost_wwpn, t->vhost_wwpn, > + sizeof(vs->vs_vhost_wwpn)); > + vs->vs_endpoint = true; > + } > + mutex_unlock(&vs->dev.mutex); > + This looks racy to me.. Why not just set these while holding vs->dev.mutex above, instead of releasing + re-aquire..? > + return 0; > } > > static int vhost_scsi_clear_endpoint( > @@ -803,7 +836,8 @@ static int vhost_scsi_clear_endpoint( > { > struct tcm_vhost_tport *tv_tport; > struct tcm_vhost_tpg *tv_tpg; > - int index, ret; > + int index, ret, i; > + u8 target; > > mutex_lock(&vs->dev.mutex); > /* Verify that ring has been setup correctly. */ > @@ -813,27 +847,32 @@ static int vhost_scsi_clear_endpoint( > goto err; > } > } > + for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) { > + target = i; > > - if (!vs->vs_tpg) { > - ret = -ENODEV; > - goto err; > - } > - tv_tpg = vs->vs_tpg; > - tv_tport = tv_tpg->tport; > - > - if (strcmp(tv_tport->tport_name, t->vhost_wwpn) || > - (tv_tpg->tport_tpgt != t->vhost_tpgt)) { > - pr_warn("tv_tport->tport_name: %s, tv_tpg->tport_tpgt: %hu" > - " does not match t->vhost_wwpn: %s, t->vhost_tpgt: %hu\n", > - tv_tport->tport_name, tv_tpg->tport_tpgt, > - t->vhost_wwpn, t->vhost_tpgt); > - ret = -EINVAL; > - goto err; > + tv_tpg = vs->vs_tpg[target]; > + if (!tv_tpg) > + continue; > + > + tv_tport = tv_tpg->tport; > + if (!tv_tport) { > + ret = -ENODEV; > + goto err; > + } > + > + if (strcmp(tv_tport->tport_name, t->vhost_wwpn)) { > + pr_warn("tv_tport->tport_name: %s, tv_tpg->tport_tpgt: %hu" > + " does not match t->vhost_wwpn: %s, t->vhost_tpgt: %hu\n", > + tv_tport->tport_name, tv_tpg->tport_tpgt, > + t->vhost_wwpn, t->vhost_tpgt); > + ret = -EINVAL; > + goto err; > + } > + tv_tpg->tv_tpg_vhost_count--; > + vs->vs_tpg[target] = NULL; > + vs->vs_endpoint = false; > } > - tv_tpg->tv_tpg_vhost_count--; > - vs->vs_tpg = NULL; > mutex_unlock(&vs->dev.mutex); > - > return 0; > > err: > @@ -868,16 +907,12 @@ static int vhost_scsi_open(struct inode *inode, struct file *f) > static int vhost_scsi_release(struct inode *inode, struct file *f) > { > struct vhost_scsi *s = f->private_data; > + struct vhost_scsi_target t; > > - if (s->vs_tpg && s->vs_tpg->tport) { > - struct vhost_scsi_target backend; > - > - memcpy(backend.vhost_wwpn, s->vs_tpg->tport->tport_name, > - sizeof(backend.vhost_wwpn)); > - backend.vhost_tpgt = s->vs_tpg->tport_tpgt; > - vhost_scsi_clear_endpoint(s, &backend); > - } > - > + mutex_lock(&s->dev.mutex); > + memcpy(t.vhost_wwpn, s->vs_vhost_wwpn, sizeof(t.vhost_wwpn)); > + mutex_unlock(&s->dev.mutex); > + vhost_scsi_clear_endpoint(s, &t); I think vhost_scsi_release() logic still needs a check here to prevent vhost_scsi_clear_endpoint() from being called when no endpoint has been configured, no..? > vhost_dev_stop(&s->dev); > vhost_dev_cleanup(&s->dev, false); > kfree(s); > diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h > index 47ee80b..519a550 100644 > --- a/drivers/vhost/tcm_vhost.h > +++ b/drivers/vhost/tcm_vhost.h > @@ -93,9 +93,11 @@ struct tcm_vhost_tport { > * > * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate + > * RFC-v2 vhost-scsi userspace. Add GET_ABI_VERSION ioctl usage > + * ABI Rev 1: January 2013. Ignore vhost_tpgt filed in struct vhost_scsi_target. > + * All the targets under vhost_wwpn can be seen and used by guset. > */ > > -#define VHOST_SCSI_ABI_VERSION 0 > +#define VHOST_SCSI_ABI_VERSION 1 > So Paolo's preference was to not bump the ABI rev for this change. I don't really have a preference either way, so I'll defer to his and MST's judgment here. --nab -- 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 02/05/2013 04:48 AM, Nicholas A. Bellinger wrote: > Hi Asias, > > On Fri, 2013-02-01 at 16:16 +0800, Asias He wrote: >> In order to take advantages of Paolo's multi-queue virito-scsi, we need >> multi-target support in tcm_vhost first. Otherwise all the requests go >> to one queue and other queues are idle. >> >> This patch makes: >> >> 1. All the targets under the wwpn is seen and can be used by guest. >> 2. No need to pass the tpgt number in struct vhost_scsi_target to >> tcm_vhost.ko. Only wwpn is needed. >> 3. We can always pass max_target = 255 to guest now, since we abort the >> request who's target id does not exist. >> >> Changes in v2: >> - Handle non-contiguous tpgt >> >> Signed-off-by: Asias He <asias@redhat.com> >> --- > > So this patch is looks pretty good. Just a few points below.. Thanks. > >> drivers/vhost/tcm_vhost.c | 117 ++++++++++++++++++++++++++++++---------------- >> drivers/vhost/tcm_vhost.h | 4 +- >> 2 files changed, 79 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c >> index 218deb6..f1481f0 100644 >> --- a/drivers/vhost/tcm_vhost.c >> +++ b/drivers/vhost/tcm_vhost.c >> @@ -59,8 +59,14 @@ enum { >> VHOST_SCSI_VQ_IO = 2, >> }; >> >> +#define VHOST_SCSI_MAX_TARGET 256 >> + >> struct vhost_scsi { >> - struct tcm_vhost_tpg *vs_tpg; /* Protected by vhost_scsi->dev.mutex */ >> + /* Protected by vhost_scsi->dev.mutex */ >> + struct tcm_vhost_tpg *vs_tpg[VHOST_SCSI_MAX_TARGET]; >> + char vs_vhost_wwpn[TRANSPORT_IQN_LEN]; >> + bool vs_endpoint; >> + >> struct vhost_dev dev; >> struct vhost_virtqueue vqs[3]; >> >> @@ -564,13 +570,11 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs) >> u32 exp_data_len, data_first, data_num, data_direction; >> unsigned out, in, i; >> int head, ret; >> + u8 target; >> >> /* Must use ioctl VHOST_SCSI_SET_ENDPOINT */ >> - tv_tpg = vs->vs_tpg; >> - if (unlikely(!tv_tpg)) { >> - pr_err("%s endpoint not set\n", __func__); >> + if (unlikely(!vs->vs_endpoint)) >> return; >> - } >> >> mutex_lock(&vq->mutex); >> vhost_disable_notify(&vs->dev, vq); >> @@ -637,6 +641,28 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs) >> break; >> } >> >> + /* Extract the tpgt */ >> + target = v_req.lun[1]; >> + tv_tpg = vs->vs_tpg[target]; >> + >> + /* Target does not exist, fail the request */ >> + if (unlikely(!tv_tpg)) { >> + struct virtio_scsi_cmd_resp __user *resp; >> + struct virtio_scsi_cmd_resp rsp; >> + >> + memset(&rsp, 0, sizeof(rsp)); >> + rsp.response = VIRTIO_SCSI_S_BAD_TARGET; >> + resp = vq->iov[out].iov_base; >> + ret = __copy_to_user(resp, &rsp, sizeof(rsp)); >> + if (!ret) >> + vhost_add_used_and_signal(&vs->dev, >> + &vs->vqs[2], head, 0); >> + else >> + pr_err("Faulted on virtio_scsi_cmd_resp\n"); >> + >> + continue; >> + } >> + >> exp_data_len = 0; >> for (i = 0; i < data_num; i++) >> exp_data_len += vq->iov[data_first + i].iov_len; >> @@ -745,6 +771,7 @@ static int vhost_scsi_set_endpoint( >> { >> struct tcm_vhost_tport *tv_tport; >> struct tcm_vhost_tpg *tv_tpg; >> + bool match = false; >> int index; >> >> mutex_lock(&vs->dev.mutex); >> @@ -771,14 +798,11 @@ static int vhost_scsi_set_endpoint( >> } >> tv_tport = tv_tpg->tport; >> >> - if (!strcmp(tv_tport->tport_name, t->vhost_wwpn) && >> - (tv_tpg->tport_tpgt == t->vhost_tpgt)) { >> + if (!strcmp(tv_tport->tport_name, t->vhost_wwpn)) { >> tv_tpg->tv_tpg_vhost_count++; >> - mutex_unlock(&tv_tpg->tv_tpg_mutex); >> - mutex_unlock(&tcm_vhost_mutex); >> >> mutex_lock(&vs->dev.mutex); >> - if (vs->vs_tpg) { >> + if (vs->vs_tpg[tv_tpg->tport_tpgt]) { >> mutex_unlock(&vs->dev.mutex); >> mutex_lock(&tv_tpg->tv_tpg_mutex); >> tv_tpg->tv_tpg_vhost_count--; >> @@ -786,15 +810,24 @@ static int vhost_scsi_set_endpoint( >> return -EEXIST; >> } >> >> - vs->vs_tpg = tv_tpg; >> + vs->vs_tpg[tv_tpg->tport_tpgt] = tv_tpg; >> smp_mb__after_atomic_inc(); >> + match = true; >> mutex_unlock(&vs->dev.mutex); >> - return 0; >> } >> mutex_unlock(&tv_tpg->tv_tpg_mutex); >> } >> mutex_unlock(&tcm_vhost_mutex); >> - return -EINVAL; >> + >> + mutex_lock(&vs->dev.mutex); >> + if (match) { >> + memcpy(vs->vs_vhost_wwpn, t->vhost_wwpn, >> + sizeof(vs->vs_vhost_wwpn)); >> + vs->vs_endpoint = true; >> + } >> + mutex_unlock(&vs->dev.mutex); >> + > > This looks racy to me.. Why not just set these while holding > vs->dev.mutex above, instead of releasing + re-aquire..? I wanted to set vs->vs_endpoint after all the vs->vs_tpg[] are set. However, we can take and release the vs->dev.mutex lock once. Will do it in v3. >> + return 0; >> } >> >> static int vhost_scsi_clear_endpoint( >> @@ -803,7 +836,8 @@ static int vhost_scsi_clear_endpoint( >> { >> struct tcm_vhost_tport *tv_tport; >> struct tcm_vhost_tpg *tv_tpg; >> - int index, ret; >> + int index, ret, i; >> + u8 target; >> >> mutex_lock(&vs->dev.mutex); >> /* Verify that ring has been setup correctly. */ >> @@ -813,27 +847,32 @@ static int vhost_scsi_clear_endpoint( >> goto err; >> } >> } >> + for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) { >> + target = i; >> >> - if (!vs->vs_tpg) { >> - ret = -ENODEV; >> - goto err; >> - } >> - tv_tpg = vs->vs_tpg; >> - tv_tport = tv_tpg->tport; >> - >> - if (strcmp(tv_tport->tport_name, t->vhost_wwpn) || >> - (tv_tpg->tport_tpgt != t->vhost_tpgt)) { >> - pr_warn("tv_tport->tport_name: %s, tv_tpg->tport_tpgt: %hu" >> - " does not match t->vhost_wwpn: %s, t->vhost_tpgt: %hu\n", >> - tv_tport->tport_name, tv_tpg->tport_tpgt, >> - t->vhost_wwpn, t->vhost_tpgt); >> - ret = -EINVAL; >> - goto err; >> + tv_tpg = vs->vs_tpg[target]; >> + if (!tv_tpg) >> + continue; >> + >> + tv_tport = tv_tpg->tport; >> + if (!tv_tport) { >> + ret = -ENODEV; >> + goto err; >> + } >> + >> + if (strcmp(tv_tport->tport_name, t->vhost_wwpn)) { >> + pr_warn("tv_tport->tport_name: %s, tv_tpg->tport_tpgt: %hu" >> + " does not match t->vhost_wwpn: %s, t->vhost_tpgt: %hu\n", >> + tv_tport->tport_name, tv_tpg->tport_tpgt, >> + t->vhost_wwpn, t->vhost_tpgt); >> + ret = -EINVAL; >> + goto err; >> + } >> + tv_tpg->tv_tpg_vhost_count--; >> + vs->vs_tpg[target] = NULL; >> + vs->vs_endpoint = false; >> } >> - tv_tpg->tv_tpg_vhost_count--; >> - vs->vs_tpg = NULL; >> mutex_unlock(&vs->dev.mutex); >> - >> return 0; >> >> err: >> @@ -868,16 +907,12 @@ static int vhost_scsi_open(struct inode *inode, struct file *f) >> static int vhost_scsi_release(struct inode *inode, struct file *f) >> { >> struct vhost_scsi *s = f->private_data; >> + struct vhost_scsi_target t; >> >> - if (s->vs_tpg && s->vs_tpg->tport) { >> - struct vhost_scsi_target backend; >> - >> - memcpy(backend.vhost_wwpn, s->vs_tpg->tport->tport_name, >> - sizeof(backend.vhost_wwpn)); >> - backend.vhost_tpgt = s->vs_tpg->tport_tpgt; >> - vhost_scsi_clear_endpoint(s, &backend); >> - } >> - >> + mutex_lock(&s->dev.mutex); >> + memcpy(t.vhost_wwpn, s->vs_vhost_wwpn, sizeof(t.vhost_wwpn)); >> + mutex_unlock(&s->dev.mutex); >> + vhost_scsi_clear_endpoint(s, &t); > > I think vhost_scsi_release() logic still needs a check here to prevent > vhost_scsi_clear_endpoint() from being called when no endpoint has been > configured, no..? Yes. we need the check, but it is done in vhost_scsi_clear_endpoint now. --> tv_tpg = vs->vs_tpg[target]; if (!tv_tpg) continue; tv_tport = tv_tpg->tport; if (!tv_tport) { ret = -ENODEV; goto err; } >> vhost_dev_stop(&s->dev); >> vhost_dev_cleanup(&s->dev, false); >> kfree(s); >> diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h >> index 47ee80b..519a550 100644 >> --- a/drivers/vhost/tcm_vhost.h >> +++ b/drivers/vhost/tcm_vhost.h >> @@ -93,9 +93,11 @@ struct tcm_vhost_tport { >> * >> * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate + >> * RFC-v2 vhost-scsi userspace. Add GET_ABI_VERSION ioctl usage >> + * ABI Rev 1: January 2013. Ignore vhost_tpgt filed in struct vhost_scsi_target. >> + * All the targets under vhost_wwpn can be seen and used by guset. >> */ >> >> -#define VHOST_SCSI_ABI_VERSION 0 >> +#define VHOST_SCSI_ABI_VERSION 1 >> > > So Paolo's preference was to not bump the ABI rev for this change. I > don't really have a preference either way, so I'll defer to his and > MST's judgment here. I am ok with either way also.
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c index 218deb6..f1481f0 100644 --- a/drivers/vhost/tcm_vhost.c +++ b/drivers/vhost/tcm_vhost.c @@ -59,8 +59,14 @@ enum { VHOST_SCSI_VQ_IO = 2, }; +#define VHOST_SCSI_MAX_TARGET 256 + struct vhost_scsi { - struct tcm_vhost_tpg *vs_tpg; /* Protected by vhost_scsi->dev.mutex */ + /* Protected by vhost_scsi->dev.mutex */ + struct tcm_vhost_tpg *vs_tpg[VHOST_SCSI_MAX_TARGET]; + char vs_vhost_wwpn[TRANSPORT_IQN_LEN]; + bool vs_endpoint; + struct vhost_dev dev; struct vhost_virtqueue vqs[3]; @@ -564,13 +570,11 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs) u32 exp_data_len, data_first, data_num, data_direction; unsigned out, in, i; int head, ret; + u8 target; /* Must use ioctl VHOST_SCSI_SET_ENDPOINT */ - tv_tpg = vs->vs_tpg; - if (unlikely(!tv_tpg)) { - pr_err("%s endpoint not set\n", __func__); + if (unlikely(!vs->vs_endpoint)) return; - } mutex_lock(&vq->mutex); vhost_disable_notify(&vs->dev, vq); @@ -637,6 +641,28 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs) break; } + /* Extract the tpgt */ + target = v_req.lun[1]; + tv_tpg = vs->vs_tpg[target]; + + /* Target does not exist, fail the request */ + if (unlikely(!tv_tpg)) { + struct virtio_scsi_cmd_resp __user *resp; + struct virtio_scsi_cmd_resp rsp; + + memset(&rsp, 0, sizeof(rsp)); + rsp.response = VIRTIO_SCSI_S_BAD_TARGET; + resp = vq->iov[out].iov_base; + ret = __copy_to_user(resp, &rsp, sizeof(rsp)); + if (!ret) + vhost_add_used_and_signal(&vs->dev, + &vs->vqs[2], head, 0); + else + pr_err("Faulted on virtio_scsi_cmd_resp\n"); + + continue; + } + exp_data_len = 0; for (i = 0; i < data_num; i++) exp_data_len += vq->iov[data_first + i].iov_len; @@ -745,6 +771,7 @@ static int vhost_scsi_set_endpoint( { struct tcm_vhost_tport *tv_tport; struct tcm_vhost_tpg *tv_tpg; + bool match = false; int index; mutex_lock(&vs->dev.mutex); @@ -771,14 +798,11 @@ static int vhost_scsi_set_endpoint( } tv_tport = tv_tpg->tport; - if (!strcmp(tv_tport->tport_name, t->vhost_wwpn) && - (tv_tpg->tport_tpgt == t->vhost_tpgt)) { + if (!strcmp(tv_tport->tport_name, t->vhost_wwpn)) { tv_tpg->tv_tpg_vhost_count++; - mutex_unlock(&tv_tpg->tv_tpg_mutex); - mutex_unlock(&tcm_vhost_mutex); mutex_lock(&vs->dev.mutex); - if (vs->vs_tpg) { + if (vs->vs_tpg[tv_tpg->tport_tpgt]) { mutex_unlock(&vs->dev.mutex); mutex_lock(&tv_tpg->tv_tpg_mutex); tv_tpg->tv_tpg_vhost_count--; @@ -786,15 +810,24 @@ static int vhost_scsi_set_endpoint( return -EEXIST; } - vs->vs_tpg = tv_tpg; + vs->vs_tpg[tv_tpg->tport_tpgt] = tv_tpg; smp_mb__after_atomic_inc(); + match = true; mutex_unlock(&vs->dev.mutex); - return 0; } mutex_unlock(&tv_tpg->tv_tpg_mutex); } mutex_unlock(&tcm_vhost_mutex); - return -EINVAL; + + mutex_lock(&vs->dev.mutex); + if (match) { + memcpy(vs->vs_vhost_wwpn, t->vhost_wwpn, + sizeof(vs->vs_vhost_wwpn)); + vs->vs_endpoint = true; + } + mutex_unlock(&vs->dev.mutex); + + return 0; } static int vhost_scsi_clear_endpoint( @@ -803,7 +836,8 @@ static int vhost_scsi_clear_endpoint( { struct tcm_vhost_tport *tv_tport; struct tcm_vhost_tpg *tv_tpg; - int index, ret; + int index, ret, i; + u8 target; mutex_lock(&vs->dev.mutex); /* Verify that ring has been setup correctly. */ @@ -813,27 +847,32 @@ static int vhost_scsi_clear_endpoint( goto err; } } + for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) { + target = i; - if (!vs->vs_tpg) { - ret = -ENODEV; - goto err; - } - tv_tpg = vs->vs_tpg; - tv_tport = tv_tpg->tport; - - if (strcmp(tv_tport->tport_name, t->vhost_wwpn) || - (tv_tpg->tport_tpgt != t->vhost_tpgt)) { - pr_warn("tv_tport->tport_name: %s, tv_tpg->tport_tpgt: %hu" - " does not match t->vhost_wwpn: %s, t->vhost_tpgt: %hu\n", - tv_tport->tport_name, tv_tpg->tport_tpgt, - t->vhost_wwpn, t->vhost_tpgt); - ret = -EINVAL; - goto err; + tv_tpg = vs->vs_tpg[target]; + if (!tv_tpg) + continue; + + tv_tport = tv_tpg->tport; + if (!tv_tport) { + ret = -ENODEV; + goto err; + } + + if (strcmp(tv_tport->tport_name, t->vhost_wwpn)) { + pr_warn("tv_tport->tport_name: %s, tv_tpg->tport_tpgt: %hu" + " does not match t->vhost_wwpn: %s, t->vhost_tpgt: %hu\n", + tv_tport->tport_name, tv_tpg->tport_tpgt, + t->vhost_wwpn, t->vhost_tpgt); + ret = -EINVAL; + goto err; + } + tv_tpg->tv_tpg_vhost_count--; + vs->vs_tpg[target] = NULL; + vs->vs_endpoint = false; } - tv_tpg->tv_tpg_vhost_count--; - vs->vs_tpg = NULL; mutex_unlock(&vs->dev.mutex); - return 0; err: @@ -868,16 +907,12 @@ static int vhost_scsi_open(struct inode *inode, struct file *f) static int vhost_scsi_release(struct inode *inode, struct file *f) { struct vhost_scsi *s = f->private_data; + struct vhost_scsi_target t; - if (s->vs_tpg && s->vs_tpg->tport) { - struct vhost_scsi_target backend; - - memcpy(backend.vhost_wwpn, s->vs_tpg->tport->tport_name, - sizeof(backend.vhost_wwpn)); - backend.vhost_tpgt = s->vs_tpg->tport_tpgt; - vhost_scsi_clear_endpoint(s, &backend); - } - + mutex_lock(&s->dev.mutex); + memcpy(t.vhost_wwpn, s->vs_vhost_wwpn, sizeof(t.vhost_wwpn)); + mutex_unlock(&s->dev.mutex); + vhost_scsi_clear_endpoint(s, &t); vhost_dev_stop(&s->dev); vhost_dev_cleanup(&s->dev, false); kfree(s); diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h index 47ee80b..519a550 100644 --- a/drivers/vhost/tcm_vhost.h +++ b/drivers/vhost/tcm_vhost.h @@ -93,9 +93,11 @@ struct tcm_vhost_tport { * * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate + * RFC-v2 vhost-scsi userspace. Add GET_ABI_VERSION ioctl usage + * ABI Rev 1: January 2013. Ignore vhost_tpgt filed in struct vhost_scsi_target. + * All the targets under vhost_wwpn can be seen and used by guset. */ -#define VHOST_SCSI_ABI_VERSION 0 +#define VHOST_SCSI_ABI_VERSION 1 struct vhost_scsi_target { int abi_version;
In order to take advantages of Paolo's multi-queue virito-scsi, we need multi-target support in tcm_vhost first. Otherwise all the requests go to one queue and other queues are idle. This patch makes: 1. All the targets under the wwpn is seen and can be used by guest. 2. No need to pass the tpgt number in struct vhost_scsi_target to tcm_vhost.ko. Only wwpn is needed. 3. We can always pass max_target = 255 to guest now, since we abort the request who's target id does not exist. Changes in v2: - Handle non-contiguous tpgt Signed-off-by: Asias He <asias@redhat.com> --- drivers/vhost/tcm_vhost.c | 117 ++++++++++++++++++++++++++++++---------------- drivers/vhost/tcm_vhost.h | 4 +- 2 files changed, 79 insertions(+), 42 deletions(-)