Re: [Qemu-devel] [PATCH v4 4/5] 9p: stat_to_qid: implement slow path

[Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org>]

On Freitag, 28. Juni 2019 12:21:20 CEST Greg Kurz wrote:
> > +static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
> > +                            uint64_t *path)
> > +{
> > +    QpfEntry lookup = {
> > +        .dev = stbuf->st_dev,
> > +        .ino = stbuf->st_ino
> > +    }, *val;
> > +    uint32_t hash = qpf_hash(lookup);
> > +
> > +    /* most users won't need the fullmap, so init the table lazily */
> > +    if (!pdu->s->qpf_table.map) {
> > +        qht_init(&pdu->s->qpf_table, qpf_lookup_func, 1 << 16,
> > QHT_MODE_AUTO_RESIZE); +    }
> > +
> > +    val = qht_lookup(&pdu->s->qpf_table, &lookup, hash);
> > +
> > +    if (!val) {
> > +        if (pdu->s->qp_fullpath_next == 0) {
> > +            /* no more files can be mapped :'( */
> 
> This would be the place to put the error_report_once() suggested
> in the previous patch actually.

I will add the suggested error message to qid_path_prefixmap() in patch 3 and 
then will move over that error message to qid_path_fullmap() in patch 4.

Or if you want I can also leave an error_report_once() in qid_path_prefixmap() 
in patch 4 about potential degraded performance.

> > @@ -3779,7 +3831,8 @@ void v9fs_device_unrealize_common(V9fsState *s,
> > Error **errp)> 
> >      }
> >      fsdev_throttle_cleanup(s->ctx.fst);
> >      g_free(s->tag);
> > 
> > -    qpp_table_destroy(&s->qpp_table);
> > +    qp_table_destroy(&s->qpp_table);
> > +    qp_table_destroy(&s->qpf_table);
> 
> I'm starting to think v9fs_device_unrealize_common() should be made
> idempotent, so that it can be used to handle rollback of a partially
> realized device, and thus avoid the code duplication. But this is
> out-of-scope for this series.

Well, I can also make that e.g.:

	if (s->qpf_table.map)
		qp_table_destroy(&s->qpf_table);

if you prefer the occurrence amount to be reduced.

Best regards,
Christian Schoenebeck